Bug 80677 - LIMITS_H_TEST is wrong
Summary: LIMITS_H_TEST is wrong
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: bootstrap (show other bugs)
Version: 7.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-08 18:09 UTC by Helmut Grohne
Modified: 2022-12-01 16:13 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
eliminate the need for LIMITS_H_TEST entirely (543 bytes, patch)
2022-11-05 12:53 UTC, Helmut Grohne
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helmut Grohne 2017-05-08 18:09:49 UTC
LIMITS_H_TEST is a Makefile variable defined in gcc/Makefile.in, that determines how to generate its own limits.h, in particular whether to use limitx.h and limity.h. The test simply tests whether $(BUILD_SYSTEM_HEADER_DIR)/limits.h exists and for most practical purposes this tests whether /usr/include/limits.h exists.

When the build and target architectures equal, this is fine. When they don't bad things happen.

False positives: When building on a typical GNU/Linux system for a baremetal target, the test indicates wrongly indicates success.

False negatives: Debian is about to further multiarch. That involves moving libc headers from /usr/include to /usr/include/$(DEB_HOST_MULTIARCH) as libc headers can differ for different libc implementations (glibc/musl/uclibc). Thus the test will wrongly fail even for libcs that provide a limits.h.

It seems that the false positive is present since ages and nobody ever noticed. Thus it probably is harmless.

The false negative generates a limits.h that disagrees on MB_LEN_MAX with glibc and breaks builds. (# error "Assumed value of MB_LEN_MAX wrong" when including <stdlib.h> after <limits.h>)

Thus I propose setting "LIMITS_H_TEST = :" (i.e. always assuming limits.h presence) as an improved heuristic. I also tried invoking $(GCC_FOR_TARGET) -E to check for limits.h presence, but since configure.ac overwrites the GCC_FOR_TARGET defined in gcc/Makefile.in, the required -isystem flags are missing and it has no chance of finding the header.
Comment 1 jsm-csl@polyomino.org.uk 2017-05-08 21:31:21 UTC
On Mon, 8 May 2017, helmut at subdivi dot de wrote:

> False negatives: Debian is about to further multiarch. That involves moving
> libc headers from /usr/include to /usr/include/$(DEB_HOST_MULTIARCH) as libc
> headers can differ for different libc implementations (glibc/musl/uclibc). Thus
> the test will wrongly fail even for libcs that provide a limits.h.

Well, if headers move then configure (and related) tests that look at them 
will need updating.  See how gcc/configure.ac looks in $target_header_dir 
to identify the glibc version and various other configuration, for 
example.
Comment 2 Helmut Grohne 2017-05-09 04:01:39 UTC
(In reply to joseph@codesourcery.com from comment #1)
> Well, if headers move then configure (and related) tests that look at them 
> will need updating.  See how gcc/configure.ac looks in $target_header_dir 
> to identify the glibc version and various other configuration, for 
> example.

As far as I understand it, gcc's build system takes care to consult $(build_tooldir)/sys-include. Debian's packaging of gcc takes care to populate it reasonably.

I have performed a fair number of builds of gcc with glibc's headers moved now and cannot confirm the projected behavior. At present, it looks like fixing LIMITS_H_TEST is the remaining piece to the puzzle.
Comment 3 Helmut Grohne 2022-11-05 12:53:03 UTC
Created attachment 53832 [details]
eliminate the need for LIMITS_H_TEST entirely

Rather than fix LIMITS_H_TEST, I now propose deleting it. The check that we try to perform at build time with wrong paths can be deferred to runtime and then operated with correct paths. I'm using this patch since at least gcc-11 and only encountered one regression thus far. It triggers https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80755 on hurd, but this is a plain gcc bug that needs fixing anyway, so this should be good to go.