On 2022-10-26 07:09, Steve Summit via tz wrote:
The core dump occurred because GCC translates this: qsort(links, nlinks, sizeof *links, qsort_linkcmp); as if it were this:
if (nlinks == 0) __builtin_trap(); qsort(links, nlinks, sizeof *links, qsort_linkcmp); This is an astonishing result. It's hard to imagine it's worthwhile for gcc to perform this "optimization".
Oh, sorry, it's astonishing because I misread the assembly language that GCC generates. The actual translation looks at links, not nlinks. That is, it's translated as if it were: if (links == NULL) __builtin_trap(); qsort(links, nlinks, sizeof *links, qsort_linkcmp); Sorry about the confusion.
Paul, which version of gcc is this, and what is $(GCC_DEBUG_FLAGS) expanding to?
It's GCC 12.2.1 20220819 (Red Hat 12.2.1-2). $(GCC_DEBUG_FLAGS) is the default in the Makefile, which expands to the long string at the end of this email. The key option here is -fsanitize=undefined, which causes GCC to insert extra checking code to trap in many (but not all) places when behavior is undefined; this explains why GCC pessimized the code here. GCC generates the ud2 instruction because $(GCC_DEBUG_FLAGS) also contains -fsanitize-undefined-trap-on-error. I use this option, despite the hassle it causes when debugging, because it means I don't have to worry about libubsan which can be a configuration problem. I use these debugging options partly to check tzcode's portability. Although qsort(NULL, 0, ...) works fine on most practical platforms, as Clive noted there is (or was) an oddball platform or two that is (or was) verrrry picky about pointers, and for a program like zic where portability is more important than performance, it's nice if zic runs even on oddballs. Clive, do you happen to know what these platforms are (or were), and whether they're still supported? As a practical matter, even though the C standard says that expressions like qsort(NULL, 0, ...) and (char *)NULL + 0 have undefined behavior, any new platform would be verrrry wise to define them to work the same way that most practical platforms do; this is true regardless of what the C standard says. And to some extent this means the C standard is not at the sweetest spot it could be in, as a contract between implementers and programmers. I write and see a lot of code where adding 0 to NULL is expected to yield NULL, and nobody thinks twice about it (nor should they). Here's what $(GCC_DEBUG_FLAGS) expands to in the default build: -DTZDIR='"/usr/share/zoneinfo"' -DGCC_LINT -g3 -O3 -fno-common -fsanitize=undefined -fsanitize-address-use-after-scope -fsanitize-undefined-trap-on-error -fstack-protector -Wall -Wextra -Walloc-size-larger-than=100000 -Warray-bounds=2 -Wbad-function-cast -Wbidi-chars=any,ucn -Wcast-align=strict -Wdate-time -Wdeclaration-after-statement -Wdouble-promotion -Wduplicated-branches -Wduplicated-cond -Wformat=2 -Wformat-overflow=2 -Wformat-signedness -Wformat-truncation -Winit-self -Wlogical-op -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wnull-dereference -Wold-style-definition -Woverlength-strings -Wpointer-arith -Wshadow -Wshift-overflow=2 -Wstrict-overflow -Wstrict-prototypes -Wstringop-overflow=4 -Wstringop-truncation -Wsuggest-attribute=cold -Wsuggest-attribute=const -Wsuggest-attribute=format -Wsuggest-attribute=malloc -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wundef -Wuninitialized -Wunused-macros -Wuse-after-free=3 -Wvariadic-macros -Wvla -Wwrite-strings -Wno-address -Wno-format-nonliteral -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter