Re: [PATCH 2/2] perf symbols: Fix HAVE_LIBBFD_BUILDID_SUPPORT build

[Brendan McGrath]

On 9/5/25 01:53, Ian Rogers wrote:
> On Thu, Sep 4, 2025 at 7:18 AM James Clark <james.clark@xxxxxxxxxx> wrote:
> > On 04/09/2025 9:27 am, Rémi Bernon wrote:
> > > Hi!
> > >
> > > On 9/4/25 10:13, James Clark wrote:
> > > > On 03/09/2025 5:07 pm, Ian Rogers wrote:
> > > > > On Wed, Sep 3, 2025 at 8:15 AM James Clark <james.clark@xxxxxxxxxx>
> > > > > wrote:
> > > > > > read_build_id() now has a blocking argument, but libbfd uses fopen()
> > > > > > internally which doesn't support O_NONBLOCK. Fix the build by adding
> > > > > > the
> > > > > > argument and ignoring it:
> > > > > >
> > > > > >     util/symbol-elf.c:964:8: error: too many arguments to function
> > > > > > ‘read_build_id’
> > > > > >       964 |  err = read_build_id(filename, bid, block);
> > > > > >
> > > > > > Fixes: 2c369d91d093 ("perf symbol: Add blocking argument to
> > > > > > filename__read_build_id")
> > > > > > Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
> > > > >
> > > > > Libbfd should go away:
> > > > > https://lore.kernel.org/lkml/20250823003216.733941-14-
> > > > > irogers@xxxxxxxxxx/
> > > > > but I can imagine that currently this is hit in a build test - sorry
> > > > > for missing that and thanks for the fix!
> > > >
> > > > Yeah just one of the build tests, I'm not actually using it.
> > > >
> > > > Remi are you still using this? To be fair the addition for PE support
> > > > is fairly recent and even includes a binary for testing it so I'm not
> > > > sure if we should be so quick to remove it.
> > > Yes, I'm still using it occasionally, and I think it's generally useful
> > > for Wine profiling purposes and I would rather prefer that it's not
> > > removed.
> > >
> > > I know it's not built by default because of license conflicts. I didn't
> > > realize that was an issue when contributing the changes, and it is quite
> > > unfortunate (and silly IMO).
> > >
> > > Then I'm not particularly attached to libbfd and any other option that
> > > would let perf read PE files would be alright, as long as PE support is
> > > kept.
> > >
> > > Cheers,
> >
> > It looks like libLLVM might work. Looking at the doxygen there are vague
> > references to PE binaries around the getBuildID() function. But as
> > mentioned in the linked thread, it's huge at 100+ MB.
> >
> > WRT that thread, I think maybe re-writing some of this in Perf wouldn't
> > be so bad. Surely getting the buildID is trivial. For PE binaries it's
> > hard to tell what's supported currently, what's being used and what's
> > being done by what library or tool. addr2line, libbfd, symbols,
> > disassembly etc.
>
> I know some people who work on LLVM for Windows for the sake of having
> a Chrome build from Linux. It should be possible to migrate the libbfd
> use cases to LLVM.

Just wanted to let you know that I've been able to put together a PoC 
that does just this. It allows the pe-file-parsing test to pass using 
LLVM in place of libbfd.

If there's interest, I would be happy to try to shape this in to 
something that can be accepted upstream.

> If I remember John Levine's Linkers and Loaders
> book correctly (contents available by way of your favorite search
> engine) everything is just a variant of COFF anyway.
>
> It is a shame that the PE testing in buildid.sh (and the testing in
> general) is requiring `cc` as it'd be much nicer to have the tests in
> a form similar to the perf test workloads (e.g. perf test -w noploop).
> I don't have a good idea on how to fix this but just wanted to note
> it.
>
> I'll write a non-blocking patch for read_build_id with libbfd that
> matches what the others do and should avoid the hang in the meantime.
>
> Thanks,
> Ian