Bug 1539079 - valgrind: Missing build flags injection for libmpiwrap-*-linux.so
Summary: valgrind: Missing build flags injection for libmpiwrap-*-linux.so
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: valgrind
Version: 28
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Mark Wielaard
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: Fedora28BuildFlags
TreeView+ depends on / blocked
 
Reported: 2018-01-26 15:22 UTC by Florian Weimer
Modified: 2018-02-25 13:15 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-02-25 13:15:04 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Florian Weimer 2018-01-26 15:22:49 UTC
The library is compiled as:

/usr/lib64/openmpi/bin/mpicc -g -O -fno-omit-frame-pointer -Wall -fpic -m64 -Wno-deprecated-declarations  -fpic -shared  -o libmpiwrap-amd64-linux.so libmpiwrap_amd64_linux_so-libmpiwrap.o  

This lacks the usual complement of build and hardening flags.

Comment 1 Mark Wielaard 2018-01-26 15:51:52 UTC
(In reply to Florian Weimer from comment #0)
> The library is compiled as:
> 
> /usr/lib64/openmpi/bin/mpicc -g -O -fno-omit-frame-pointer -Wall -fpic -m64
> -Wno-deprecated-declarations  -fpic -shared  -o libmpiwrap-amd64-linux.so
> libmpiwrap_amd64_linux_so-libmpiwrap.o  
> 
> This lacks the usual complement of build and hardening flags.

Is that a problem?

The wrapper library is only used when running your program under valgrind. You either link directly with libmpiwrap or use LD_PRELOAD so all mpi functions go through libmpiwrap which has valgrind client requests embedded so valgrind knows about the data being passed around. It is as simple as possible and uses minimal c/ld flags because it is a simple pass-through library.

Comment 2 Florian Weimer 2018-01-26 15:54:33 UTC
(In reply to Mark Wielaard from comment #1)
> (In reply to Florian Weimer from comment #0)
> > The library is compiled as:
> > 
> > /usr/lib64/openmpi/bin/mpicc -g -O -fno-omit-frame-pointer -Wall -fpic -m64
> > -Wno-deprecated-declarations  -fpic -shared  -o libmpiwrap-amd64-linux.so
> > libmpiwrap_amd64_linux_so-libmpiwrap.o  
> > 
> > This lacks the usual complement of build and hardening flags.
> 
> Is that a problem?

Yes, every exception needs to be tracked and examined separately.  It is really preferable to minimize them if at all possible.

Comment 3 Mark Wielaard 2018-01-26 16:06:49 UTC
(In reply to Florian Weimer from comment #2)
> (In reply to Mark Wielaard from comment #1)
> > (In reply to Florian Weimer from comment #0)
> > > The library is compiled as:
> > > 
> > > /usr/lib64/openmpi/bin/mpicc -g -O -fno-omit-frame-pointer -Wall -fpic -m64
> > > -Wno-deprecated-declarations  -fpic -shared  -o libmpiwrap-amd64-linux.so
> > > libmpiwrap_amd64_linux_so-libmpiwrap.o  
> > > 
> > > This lacks the usual complement of build and hardening flags.
> > 
> > Is that a problem?
> 
> Yes, every exception needs to be tracked and examined separately.  It is
> really preferable to minimize them if at all possible.

OK. How would I properly track this?

Or do you feel this wrapper lib should actually have all the hardening flags applied?

Note we also explicitly don't add such flags for the tools vgpreload libaries (vgpreload_[core|drd|helgrind|massif|memcheck]-*-linux.so). We even keep the symtab and debuginfo in those because it is essential for good error reporting (e.g. mixing memcpy/memmove warnings). Is there a reason those aren't flagged?

Those use the following:

# These flags are used for building the preload shared objects (PSOs).
# The aim is to give reasonable performance but also to have good
# stack traces, since users often see stack traces extending
# into (and through) the preloads.  Also, we must use any
# -mpreferred-stack-boundary flag to build the preload shared
# objects, since that risks misaligning the client's stack and
# results in segfaults like (eg) #324050.

AM_CFLAGS_PSO_BASE = -O -g -fno-omit-frame-pointer -fno-strict-aliasing \
                     -fpic -fno-builtin @FLAG_FNO_IPA_ICF@

Comment 4 Florian Weimer 2018-01-26 16:27:22 UTC
(In reply to Mark Wielaard from comment #3)
> (In reply to Florian Weimer from comment #2)
> > (In reply to Mark Wielaard from comment #1)
> > > (In reply to Florian Weimer from comment #0)
> > > > The library is compiled as:
> > > > 
> > > > /usr/lib64/openmpi/bin/mpicc -g -O -fno-omit-frame-pointer -Wall -fpic -m64
> > > > -Wno-deprecated-declarations  -fpic -shared  -o libmpiwrap-amd64-linux.so
> > > > libmpiwrap_amd64_linux_so-libmpiwrap.o  
> > > > 
> > > > This lacks the usual complement of build and hardening flags.
> > > 
> > > Is that a problem?
> > 
> > Yes, every exception needs to be tracked and examined separately.  It is
> > really preferable to minimize them if at all possible.
> 
> OK. How would I properly track this?

There will be a Git repository on Pagure at one point, containing package/file lists for exceptions

> Or do you feel this wrapper lib should actually have all the hardening flags
> applied?

I would like to see as much as possible built with the official flags.

> Note we also explicitly don't add such flags for the tools vgpreload
> libaries (vgpreload_[core|drd|helgrind|massif|memcheck]-*-linux.so). We even
> keep the symtab and debuginfo in those because it is essential for good
> error reporting (e.g. mixing memcpy/memmove warnings). Is there a reason
> those aren't flagged?
> 
> Those use the following:
> 
> # These flags are used for building the preload shared objects (PSOs).
> # The aim is to give reasonable performance but also to have good
> # stack traces, since users often see stack traces extending
> # into (and through) the preloads.  Also, we must use any
> # -mpreferred-stack-boundary flag to build the preload shared
> # objects, since that risks misaligning the client's stack and

(I think you need to use -mrealignstack, BTW.)

> # results in segfaults like (eg) #324050.
> 
> AM_CFLAGS_PSO_BASE = -O -g -fno-omit-frame-pointer -fno-strict-aliasing \
>                      -fpic -fno-builtin @FLAG_FNO_IPA_ICF@

Interesting.  I do not see this yet because you link at least one object into those DSOs which carries annobin notes, and I'm using the total lack of annobin notes as a first indicator that something is wrong with the build flags injection.  Eventually, we are going to have PC range checks to see that all text bits have the intended flags or were built by tools to which our build flags are not applicable (such as assembler, Haskell, or Go).

Comment 5 Fedora End Of Life 2018-02-20 15:33:49 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 28 development cycle.
Changing version to '28'.

Comment 6 Mark Wielaard 2018-02-25 13:15:04 UTC
Closing since this is deliberate.
Please feel free to reopen if you need some marker/note somewhere to indicate this.


Note You need to log in before you can comment on or make changes to this bug.