#1816: segfaults on 64bit systems when build with c99 --------------------------------+------------------------------------------- Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Keywords: | --------------------------------+------------------------------------------- I recently encountered a segfault in ScheduleInterface.c, more precisely in the function
static int CCTKi_ScheduleCallFunction(void *function, t_attribute *attribute, t_sched_data *data) { /* find the timer for this function and this schedule bin */ t_timer *timer = attribute->timers; while (timer && strcmp(timer->schedule_bin, data->schedule_bin)) { timer = timer->next; }
Running in the debugger revealed that timer->schedule_bin pointed to an invalid address. Curiously, it had the top 33 (33 not a typo) bits all set. Taking the lowest 32 bits gave a valid address which pointed to a reasonable string "CCTK_INITIAL". This suggests a 32/64 bit issue. The pointer timer->schedule_bin seems to be initialized in the same function using strdup:
timer->schedule_bin = strdup (where);
strdup is not part of the c99 standard, but only Posix. Compiling with gcc --std=c99 means it is not defined in <string.h>. This means the compiler treats the occurrence of strdup as an implicit function declaration, and assumes it returns int. Thus, it will do an implicit conversion of the result from int to char*. If the highest bit of the int was set, this resulted in a 64 bit pointer with all 32 high bits set (I checked with a small test code). When the address returned by the actual strdup code linked from glibc has the top 33 bits zero, the conversion yields the correct results. Therefore, the problem is hard to reproduce, it only occurred with a test case almost exhausting my workstations memory, but frustratingly not small tests.
After this, I also found compiler warnings for ScheduleInterface.c of the type
implicit declaration of function ‘strdup’ [-Wimplicit-function- declaration] and assignment makes pointer from integer without a cast [-Wint-conversion]
Switching from --std=c99 to --std=gnu99 fixed the problem for now. However, this is a bug that might affect many users since the code compiles with --std=c99 and the compiler warnings are hidden within the thousands of other compiler warnings the ET code generates.
Also, a quick grep revealed many occurrences of strdup, although some of them where redefined as Util_Strdup. The rest might lead to segfaults on 64 bit systems with std=c99.
My findings concern the Wheeler release, I haven't had time to check the development version.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by knarf):
Thank you for the very detailed bug report. This is indeed very helpful.
As to how to fix this: if we assume that we cannot influence (to some degree) how users compile the code, what is left is either defining a strdup() ourself and thereby making sure it is always present, or consistently use Util_Strdup) instead of strdup(), essentially doing the same.
In terms of compiler options we could require std=gnu99, and also recommend -Werror-implicit-function-declaration (for gcc), but would not be enforceable and still leaves users out in the cold.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by eschnett):
We require Posix features in a few other cases as well. We should abort during the configure stage if {{{strdup}}} is missing.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by rhaas):
Wow, that was a very exhaustive bug report.
We actually do abort in the configure phase in the master branch. This was introduce in commit 12a02cc1813e2b3bbb653af85a3fc3dc438da8c9 related to ticket #1712.
Util_strdup is defined to be strdup itself as far as I know.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by rhaas):
Should this be backported?
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by eschnett):
I think it should be.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by hinder):
But not to Wheeler (ET_2014_05). Just to the current release.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by knarf):
I agree (with Ian)
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by knarf):
Has this been done?
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: new Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: | Keywords: ---------------------------------+------------------------------------------
Comment (by rhaas):
This was no yet done.
#1816: segfaults on 64bit systems when build with c99 ---------------------------------+------------------------------------------ Reporter: physik@… | Owner: Type: defect | Status: closed Priority: unset | Milestone: Component: Cactus | Version: ET_2014_05 Resolution: fixed | Keywords: ---------------------------------+------------------------------------------ Changes (by rhaas):
* status: new => closed * resolution: => fixed
Comment:
Fixed by delaying long enough so that master became the release branch.
trac@lists.einsteintoolkit.org