Page MenuHomeFreeBSD

rtld: clean up Makefile.
ClosedPublic

Authored by kib on Wed, Jan 8, 9:55 AM.

Details

Summary

Move all MD statements into $MACHINE_ARCH/Makefile.inc.
Unconditionally apply version script to rtld, it is not functional without it for long time.

Test Plan

So far a limited testing was done. Tinderbox is due, at least.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib created this revision.Wed, Jan 8, 9:55 AM
imp added a comment.Wed, Jan 8, 5:09 PM

This looks good, part from the one question I had.

libexec/rtld-elf/Makefile
84 ↗(On Diff #66481)

Is removing the non-symbol versioning support intentional?

brooks added inline comments.Wed, Jan 8, 6:57 PM
libexec/rtld-elf/Makefile
44 ↗(On Diff #66481)

-nostdlib seems like it should be here not duplicated. I'd be tempted to make this:

LDFLAGS+=	-nostdlib -e ${RTLD_ENTRY}
...
.sinclude "${RTLD_ELF_DIR}/${RTLD_ARCH}/Makefile.inc"
RTLD_ENTRY?=.rtld_start

with an overriding in the powerpc file(s).

59 ↗(On Diff #66481)

Given that sparc64 is heading for the dumpster, I'd be tempted to add -fpic below and a CFLAGS:=${CFLAGS:S/-fpic/-fPIC} in sparc64/Makefile.inc.

kib updated this revision to Diff 66517.Wed, Jan 8, 7:33 PM
kib marked 3 inline comments as done.

brooks' suggestions.

brooks accepted this revision.Wed, Jan 8, 11:16 PM

Looks good to me other than the minor issue on sparc64.

libexec/rtld-elf/sparc64/Makefile.inc
3 ↗(On Diff #66517)

I think there's a missing / at the end.

This revision is now accepted and ready to land.Wed, Jan 8, 11:16 PM
kib updated this revision to Diff 66534.Thu, Jan 9, 6:12 AM

Add missed '/'.

This revision now requires review to proceed.Thu, Jan 9, 6:12 AM
emaste accepted this revision.Thu, Jan 9, 3:54 PM
emaste added inline comments.
libexec/rtld-elf/arm/Makefile.inc
4–5 ↗(On Diff #66534)

this "placed first" comment seems confusing in that I don't see what's supposed to ensure that it's placed first. but it doesn't appear to be new in your change.

This revision is now accepted and ready to land.Thu, Jan 9, 3:54 PM
This revision was automatically updated to reflect the committed changes.