Opened 5 years ago

Closed 5 years ago

Last modified 11 months ago

#10093 closed bug (fixed)

Library configure scripts should not clobber CPPFLAGS nor LDFLAGS

Reported by: PHO Owned by:
Priority: normal Milestone: 8.0.1
Component: Build System (make) Version: 7.11
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime crash Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D663
Wiki Page:

Description

libraries/base/configure currently clobbers CPPFLAGS and LDFLAGS when --with-iconv-includes and --with-iconv-libraries are given, but really it shouldn't.

I will soon submit a patch for this.

Here's what I stumbled upon (TL;DR):

This is somewhat related to #2933. I have libiconv installed in /usr/pkg, which is a non-standard path that ld.so normally doesn't search libraries for. So I built GHC with CONF_GCC_LINKER_OPTS_STAGE{0,1,2} set to -Wl,-rpath,/usr/pkg/lib so that GHC links executables with RPATH pointing to /usr/pkg/lib.

The build went fine until it started using ghc-stage2 and haddock: they were causing segfaults in evacuate() with a probability of about 70%, which suggested a heap corruption occuring somewhere. I tried rebuilding RTS with -O0 -g, changed various parameters for GC, etc. Nothing changed.

Then I suddenly realized that libraries/base/configure was the root cause of the problem. Those configure scripts are executed with LDFLAGS being set to the value of CONF_GCC_LINKER_OPTS_STAGE2, which is -Wl,-rpath,/usr/pkg/lib in my case, but libraries/base/configure overwrites it to -L/usr/pkg/lib when --with-iconv-libraries=/usr/pkg/lib is given. On the other hand, FP_SEARCH_LIBS_PROTO(iconv) appends -liconv to $LIBS so it will be linked to any conftest executables that follow, including one which will be generated by AC_CHECK_SIZEOF().

The problem is that if libraries listed in $LIBS are in a non-standard path while rpath flags are missing (due to LDFLAGS being clobbered in this case), conftest executables cannot run even if they can be linked. And if anything goes wrong during AC_CHECK_SIZEOF(T), it considers sizeof(T) as 0 unless T is known to be an existing type:

  ...
  checking for library containing iconv... -liconv
  checking for library containing locale_charset... none required
  checking size of struct MD5Context... 0
  ...

This means SIZEOF_STRUCT_MD5CONTEXT gets defined to 0, GHC.Fingerprint.fingerprintData allocates 0 bytes on the heap, MD5Init/Update/Final corrupts the heap and then Bad Things will happen.

I have found the problem on NetBSD/amd64 but I'm sure the same thing happens on every ELF platform.

Change History (6)

comment:1 Changed 5 years ago by PHO

Differential Rev(s): Phab:D663
Status: newpatch

comment:2 Changed 5 years ago by PHO

Okay I submitted a code review to Phabricator. This is my first time using Phab so I hope I haven't made mistakes.

comment:3 Changed 5 years ago by Austin Seipp <austin@…>

In 9caf71a8d9293cfebdbb5b28e2d6a455ad126882/ghc:

Do not clobber CPPFLAGS nor LDFLAGS, fixes #10093

Summary: Append -I/-L flags to CPPFLAGS/LDFLAGS instead of clobbering.

Test Plan: Install libiconv into /some/non-standard/path. Set CONF_GCC_LINKER_OPTS_STAGE{0,1,2} to -Wl,-rpath,/some/non-standard/path/lib. And then run ./configure with arguments --with-iconv-includes=/some/non-standard/path/include and --with-iconv-libraries=/some/non-standard/path/lib

Reviewers: hvr, austin

Reviewed By: austin

Subscribers: thomie, PHO

Differential Revision: https://phabricator.haskell.org/D663

GHC Trac Issues: #10093

comment:4 Changed 5 years ago by thoughtpolice

Milestone: 7.12.1
Resolution: fixed
Status: patchclosed

Looks fine - merged, thanks!

comment:5 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:6 Changed 11 months ago by bgamari

Component: Build SystemBuild System (make)

The new Hadrian build system has been merged. Relabeling the tickets concerning the legacy make build system to prevent confusion.

Note: See TracTickets for help on using tickets.