- xtensa is the second architecture that supports
time64 inside uClibc-ng.
- Linux Kernel always uses 32bit time variables
inside `stat` structures, so there is a need
to use `st_atime`, `st_mtime` and `st_ctime` structures with the same
32bit-wide `tv_sec` and `tv_nsec` variables even if time64 is enabled.
Signed-off-by: Dmitry Chestnykh <dm.chestnykh(a)gmail.com>
---
extra/Configs/Config.in | 2 +-
libc/sysdeps/linux/xtensa/bits/kernel_stat.h | 22 ++++++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/extra/Configs/Config.in b/extra/Configs/Config.in
index 831b9f689..9351dffc8 100644
--- a/extra/Configs/Config.in
+++ b/extra/Configs/Config.in
@@ -1026,7 +1026,7 @@ config UCLIBC_FALLBACK_TO_ETC_LOCALTIME
config UCLIBC_USE_TIME64
bool "Use *time64 syscalls instead of 32bit ones (if possible)"
- depends on TARGET_arm
+ depends on TARGET_arm || TARGET_xtensa
# TODO: add support for other architectures
default n
diff --git a/libc/sysdeps/linux/xtensa/bits/kernel_stat.h b/libc/sysdeps/linux/xtensa/bits/kernel_stat.h
index d884344d3..80ccdb76a 100644
--- a/libc/sysdeps/linux/xtensa/bits/kernel_stat.h
+++ b/libc/sysdeps/linux/xtensa/bits/kernel_stat.h
@@ -5,6 +5,16 @@
* struct kernel_stat should look like... It turns out each arch has a
* different opinion on the subject... */
+#if defined(__UCLIBC_USE_TIME64__)
+#include <bits/types.h>
+
+struct ts32_struct {
+ __S32_TYPE tv_sec;
+ __S32_TYPE tv_nsec;
+};
+
+#endif
+
struct kernel_stat {
unsigned long st_dev;
unsigned long st_ino;
@@ -16,9 +26,15 @@ struct kernel_stat {
long st_size;
unsigned long st_blksize;
unsigned long st_blocks;
+#if defined(__UCLIBC_USE_TIME64__)
+ struct ts32_struct __st_atim32;
+ struct ts32_struct __st_mtim32;
+ struct ts32_struct __st_ctim32;
+#else
struct timespec st_atim;
struct timespec st_mtim;
struct timespec st_ctim;
+#endif
unsigned long __unused4;
unsigned long __unused5;
};
@@ -35,9 +51,15 @@ struct kernel_stat64 {
unsigned long st_blksize; /* Optimal block size for I/O. */
unsigned long __uclibc_unused2;
unsigned long long st_blocks; /* Number 512-byte blocks allocated. */
+#if defined(__UCLIBC_USE_TIME64__)
+ struct ts32_struct __st_atim32;
+ struct ts32_struct __st_mtim32;
+ struct ts32_struct __st_ctim32;
+#else
struct timespec st_atim; /* Time of last access. */
struct timespec st_mtim; /* Time of last modification. */
struct timespec st_ctim; /* Time of last status change. */
+#endif
unsigned long __uclibc_unused4;
unsigned long __uclibc_unused5;
};
--
2.43.2
Hi,
static analysis tools complain that the following code lacks a null-pointer
check:
ldso/ldso/dl-elf.c:
/*
* Add this object into the symbol chain
*/
if (*rpnt
#ifdef __LDSO_STANDALONE_SUPPORT__
/* Do not create a new chain entry for the main executable */
&& (*rpnt)->dyn
#endif
) {
(*rpnt)->next = _dl_malloc(sizeof(struct dyn_elf));
_dl_memset((*rpnt)->next, 0, sizeof(struct dyn_elf));
(*rpnt)->next->prev = (*rpnt);
*rpnt = (*rpnt)->next;
}
#ifndef SHARED
/* When statically linked, the first time we dlopen a DSO
* the *rpnt is NULL, so we need to allocate memory for it,
* and initialize the _dl_symbol_table.
*/
else {
*rpnt = _dl_symbol_tables = _dl_malloc(sizeof(struct dyn_elf));
_dl_memset(*rpnt, 0, sizeof(struct dyn_elf));
}
#endif
(*rpnt)->dyn = tpnt;
^^^^^^^^^^^^^^^^^^^^
There is a check for (*rpnt == NULL) right after the first comment but the
"else" case which performs an allocation does only exist if SHARED is not
defined. Otherwise it may happen (at least in theory) that *rpnt=NULL when
executing
(*rpnt)->dyn = tpnt;
Proposed fix:
diff --git a/ldso/ldso/dl-elf.c b/ldso/ldso/dl-elf.c
index 8210a012e..3ba3144e2 100644
--- a/ldso/ldso/dl-elf.c
+++ b/ldso/ldso/dl-elf.c
@@ -900,7 +900,8 @@ struct elf_resolve *_dl_load_elf_shared_library(unsigned int rflags,
_dl_memset(*rpnt, 0, sizeof(struct dyn_elf));
}
#endif
- (*rpnt)->dyn = tpnt;
+ if (*rpnt)
+ (*rpnt)->dyn = tpnt;
tpnt->usage_count++;
if (tpnt->rtld_flags & RTLD_NODELETE)
tpnt->usage_count++;
Kind regards
Frank
Hi,
I stumbled upon the following code in tempname.c:
switch (kind) {
case __GT_NOCREATE:
{
struct stat st;
if (stat (tmpl, &st) < 0) {
if (errno == ENOENT) {
fd = 0;
goto restore_and_ret;
} else
/* Give up now. */
return -1;
} else
fd = 0;
}
case __GT_FILE:
fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL | flags,
mode);
break;
I am baffled by the 'else' branch of the check of 'stat (tmpl, &st) <
0'.
If I understand the whole thing correctly, this is an error case,
where
a file with the temporary name already exists. But instead, the whole
execution falls through to the __GT_FILE branch, and actually opens the
file.
If this happens, this file never seems to be closed again (see for
example
the usage in tmpnam_r.c:30:
char * tmpnam_r (char *s)
{
if (s == NULL)
return NULL;
if (__path_search (s, L_tmpnam, NULL, NULL, 0))
return NULL;
if (__gen_tempname (s, __GT_NOCREATE, 0, 0, 0))
return NULL;
return s;
}
I would suppose that instead of falling to the next switch-case, we
should return with a non-zero value (maybe '1' to distinguish it from
the general error case?).
Am I missing something basic?
Cheers,
Sven
--
Sven Linker
Tel.: +49 351 41883243
Kernkonzept GmbH at Dresden, Germany,
HRB 31129, CEO Dr.-Ing. Michael Hohmuth
