This is an archive of the discontinued LLVM Phabricator instance.

Differential D107388

Work around non-existence of ElfW(type) macro on FreeBSD
ClosedPublic

Authored by dim on Aug 3 2021, 12:36 PM.

Details

Reviewers
emaste
gulfem
davidxl
phosek
Commits
rG440d9712ebf6: Work around non-existence of ElfW(type) macro on FreeBSD
Summary

Fixes PR51331. On FreeBSD, the elf headers don't (yet) provide the
ElfW(type) macro. However, there is a similar set of macros in the
<sys/elf-generic.h> header, of which __ElfN(type) exactly matches the
indended purpose.

Diff Detail

Event Timeline

dim created this revision.Aug 3 2021, 12:36 PM
Herald added subscribers: krytarowski, arichardson. · View Herald TranscriptAug 3 2021, 12:36 PM
dim requested review of this revision.Aug 3 2021, 12:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2021, 12:36 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
gulfem accepted this revision.Aug 3 2021, 1:00 PM
This revision is now accepted and ready to land.Aug 3 2021, 1:00 PM
Harbormaster completed remote builds in B117714: Diff 363837.Aug 3 2021, 1:04 PM
MaskRay added a subscriber: MaskRay.Aug 3 2021, 1:17 PM
MaskRay added inline comments.
compiler-rt/lib/profile/InstrProfilingPlatformLinux.c
21

If FreeBSD will have the macro, add a comment so that this chunk can be removed in the future.

dim added inline comments.Aug 3 2021, 1:35 PM
compiler-rt/lib/profile/InstrProfilingPlatformLinux.c
21

Newer versions might have it, but older released versions won't. That's what the !defined(ElfW) is for. But you're right that a comment is appropriate.

dim updated this revision to Diff 363875.Aug 3 2021, 1:54 PM

Add comment about the FreeBSD ElfW(type) workaround.

Harbormaster completed remote builds in B117744: Diff 363875.Aug 3 2021, 1:54 PM
dim updated this revision to Diff 363886.Aug 3 2021, 3:04 PM

Squash commits so buildkite doesn't choke.

Harbormaster completed remote builds in B117753: Diff 363886.Aug 3 2021, 3:32 PM
Closed by commit rG440d9712ebf6: Work around non-existence of ElfW(type) macro on FreeBSD (authored by dim). · Explain WhyAug 4 2021, 11:16 AM
This revision was automatically updated to reflect the committed changes.
dim added a commit: rG440d9712ebf6: Work around non-existence of ElfW(type) macro on FreeBSD.
MaskRay added inline comments.Aug 4 2021, 11:28 AM
compiler-rt/lib/profile/InstrProfilingPlatformLinux.c
23

(I expected an exact version, instead of a vague "If this is added to all supported FreeBSD versions in the future")

Without the comment, a reader may remember to clean up this as well.

dim added inline comments.Aug 4 2021, 11:43 AM
compiler-rt/lib/profile/InstrProfilingPlatformLinux.c
23

Well, predictions are hard, especially about the future. :)

That said, if we add this macro to FreeBSD 14.0-CURRENT, it can be merged back to the existing stable branches, which are currently:

  • 11.4-STABLE (but this expires in 2 months so there isn't going be a 11.5-RELEASE)
  • 12.3-STABLE, which will likely result in 12.3-RELEASE at some point, but the date is not yet determined
  • 13.1-STABLE, which will likely result in 13.1-RELEASE at some point, but the date is not yet determined

This type of addition is usually not added to existing releases, which typically only provide security fixes, and bug fixes that affect a large number of users.

Revision Contents

PathSize
compiler-rt/
lib/
profile/
9 lines

Diff 364198

compiler-rt/lib/profile/InstrProfilingPlatformLinux.c

Show All 11 Lines
#include <elf.h> #include <elf.h>
#include <link.h> #include <link.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include "InstrProfiling.h" #include "InstrProfiling.h"
#include "InstrProfilingInternal.h" #include "InstrProfilingInternal.h"
#if defined(__FreeBSD__) && !defined(ElfW)
/*
MaskRayUnsubmitted
Not Done
ReplyInline Actions

If FreeBSD will have the macro, add a comment so that this chunk can be removed in the future.

MaskRay: If FreeBSD will have the macro, add a comment so that this chunk can be removed in the future.
dimAuthorUnsubmitted
Done
ReplyInline Actions

Newer versions might have it, but older released versions won't. That's what the !defined(ElfW) is for. But you're right that a comment is appropriate.

dim: Newer versions might have it, but older released versions won't. That's what the !defined(ElfW)…
* FreeBSD's elf.h and link.h headers do not define the ElfW(type) macro yet.
* If this is added to all supported FreeBSD versions in the future, this
MaskRayUnsubmitted
Not Done
ReplyInline Actions

(I expected an exact version, instead of a vague "If this is added to all supported FreeBSD versions in the future")

Without the comment, a reader may remember to clean up this as well.

MaskRay: (I expected an exact version, instead of a vague "If this is added to all supported FreeBSD…
dimAuthorUnsubmitted
Done
ReplyInline Actions

Well, predictions are hard, especially about the future. :)

That said, if we add this macro to FreeBSD 14.0-CURRENT, it can be merged back to the existing stable branches, which are currently:

  • 11.4-STABLE (but this expires in 2 months so there isn't going be a 11.5-RELEASE)
  • 12.3-STABLE, which will likely result in 12.3-RELEASE at some point, but the date is not yet determined
  • 13.1-STABLE, which will likely result in 13.1-RELEASE at some point, but the date is not yet determined

This type of addition is usually not added to existing releases, which typically only provide security fixes, and bug fixes that affect a large number of users.

dim: Well, predictions are hard, especially about the future. :) That said, if we add this macro to…
* compatibility macro can be removed.
*/
#define ElfW(type) __ElfN(type)
#endif
#define PROF_DATA_START INSTR_PROF_SECT_START(INSTR_PROF_DATA_COMMON) #define PROF_DATA_START INSTR_PROF_SECT_START(INSTR_PROF_DATA_COMMON)
#define PROF_DATA_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_DATA_COMMON) #define PROF_DATA_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_DATA_COMMON)
#define PROF_NAME_START INSTR_PROF_SECT_START(INSTR_PROF_NAME_COMMON) #define PROF_NAME_START INSTR_PROF_SECT_START(INSTR_PROF_NAME_COMMON)
#define PROF_NAME_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_NAME_COMMON) #define PROF_NAME_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_NAME_COMMON)
#define PROF_CNTS_START INSTR_PROF_SECT_START(INSTR_PROF_CNTS_COMMON) #define PROF_CNTS_START INSTR_PROF_SECT_START(INSTR_PROF_CNTS_COMMON)
#define PROF_CNTS_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_CNTS_COMMON) #define PROF_CNTS_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_CNTS_COMMON)
#define PROF_ORDERFILE_START INSTR_PROF_SECT_START(INSTR_PROF_ORDERFILE_COMMON) #define PROF_ORDERFILE_START INSTR_PROF_SECT_START(INSTR_PROF_ORDERFILE_COMMON)
#define PROF_VNODES_START INSTR_PROF_SECT_START(INSTR_PROF_VNODES_COMMON) #define PROF_VNODES_START INSTR_PROF_SECT_START(INSTR_PROF_VNODES_COMMON)
▲ Show 20 Lines • Show All 156 Lines • Show Last 20 Lines