buildroot.busybox.net archive mirror
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
@ 2022-09-27 22:11 Florian Fainelli
  2022-09-28 21:38 ` Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Florian Fainelli @ 2022-09-27 22:11 UTC (permalink / raw)
  To: buildroot; +Cc: Florian Fainelli

Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
("perf tests: Add test for PE binary format support") present in >=
v5.10 there is an unconditional installation of PE binaries which will
be rejected by the check-bin-arch script.

Make sure that these binaries are excluded from being checked to allow
the installation of the perf tests.

Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 package/linux-tools/linux-tool-perf.mk.in | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in
index dda63cccecb4..c22097316264 100644
--- a/package/linux-tools/linux-tool-perf.mk.in
+++ b/package/linux-tools/linux-tool-perf.mk.in
@@ -169,6 +169,10 @@ define PERF_INSTALL_REMOVE_SCRIPTS
 	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/scripts/
 	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/tests/
 endef
+
+LINUX_TOOLS_BIN_ARCH_EXCLUDE += \
+	/usr/libexec/perf-core/tests/pe-file.exe \
+	/usr/libexec/perf-core/tests/pe-file.exe.debug
 endif
 
 define PERF_INSTALL_TARGET_CMDS
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-27 22:11 [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test Florian Fainelli
@ 2022-09-28 21:38 ` Thomas Petazzoni
  2022-09-28 22:28   ` Florian Fainelli
  2023-04-17 19:58 ` Yann E. MORIN
  2023-04-23 10:40 ` Peter Korsgaard
  2 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2022-09-28 21:38 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: buildroot

Hello Florian,

On Tue, 27 Sep 2022 15:11:33 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
> ("perf tests: Add test for PE binary format support") present in >=
> v5.10 there is an unconditional installation of PE binaries which will
> be rejected by the check-bin-arch script.
> 
> Make sure that these binaries are excluded from being checked to allow
> the installation of the perf tests.
> 
> Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks for the patch! Before merging it, I'd like to understand a
little bit more how readelf behaves for these PE files. Indeed in
check-bin-arch, we are doing:

        arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
                       sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)

        # If no architecture found, assume it was not an ELF file
        if test "${arch}" = "" ; then
                continue
        fi

for a PE file, I would expect readelf to badly fail, and therefore
${arch} to be empty, and the file simply ignored.

What is the behavior/output of readelf on these PE files?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-28 21:38 ` Thomas Petazzoni
@ 2022-09-28 22:28   ` Florian Fainelli
  2022-09-29  6:40     ` Thomas Petazzoni
  2022-10-01 19:07     ` Yann E. MORIN
  0 siblings, 2 replies; 11+ messages in thread
From: Florian Fainelli @ 2022-09-28 22:28 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: buildroot

On 9/28/22 14:38, Thomas Petazzoni wrote:
> Hello Florian,
> 
> On Tue, 27 Sep 2022 15:11:33 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
>> ("perf tests: Add test for PE binary format support") present in >=
>> v5.10 there is an unconditional installation of PE binaries which will
>> be rejected by the check-bin-arch script.
>>
>> Make sure that these binaries are excluded from being checked to allow
>> the installation of the perf tests.
>>
>> Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Thanks for the patch! Before merging it, I'd like to understand a
> little bit more how readelf behaves for these PE files. Indeed in
> check-bin-arch, we are doing:
> 
>          arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
>                         sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)
> 
>          # If no architecture found, assume it was not an ELF file
>          if test "${arch}" = "" ; then
>                  continue
>          fi
> 
> for a PE file, I would expect readelf to badly fail, and therefore
> ${arch} to be empty, and the file simply ignored.
> 
> What is the behavior/output of readelf on these PE files?

If I use my host system readelf which is packaged from 
binutils-x86-64-linux-gnu, I get no output and all is well, however when 
check-bin-arch is called and it uses $(TARGET_READELF), I do get:

./host/bin/aarch64-linux-readelf -h 
build/linux-custom/tools/perf/tests/pe-file.exe | sed -r -e '/^ 
Machine: +(.+)/!d; s//\1/;' | head -1
IMAGE_FILE_MACHINE_AMD64 (0x8664)

which is what prompted me to issue this patch in the first place.

I should mention that the readelf binary in this case is the LLVM Object 
Reader and it does support PE/COFF which is probably why it even 
remotely attempts to parse the file.
-- 
Florian
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-28 22:28   ` Florian Fainelli
@ 2022-09-29  6:40     ` Thomas Petazzoni
  2022-09-30 22:05       ` Florian Fainelli
  2022-10-01 19:07     ` Yann E. MORIN
  1 sibling, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2022-09-29  6:40 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: buildroot

On Wed, 28 Sep 2022 15:28:58 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> ./host/bin/aarch64-linux-readelf -h 
> build/linux-custom/tools/perf/tests/pe-file.exe | sed -r -e '/^ 
> Machine: +(.+)/!d; s//\1/;' | head -1
> IMAGE_FILE_MACHINE_AMD64 (0x8664)

Could you provide the full output? In other words, I'm interested to
see if it's really readelf showing the same "Machine:" field as for
regular ELF files, or if it's something somewhat different that we
could distinguish.

> I should mention that the readelf binary in this case is the LLVM Object 
> Reader and it does support PE/COFF which is probably why it even 
> remotely attempts to parse the file.

How come your readelf is from LLVM? Are you using an external toolchain
that isn't based on the standard GNU Binutils?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-29  6:40     ` Thomas Petazzoni
@ 2022-09-30 22:05       ` Florian Fainelli
  0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2022-09-30 22:05 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: buildroot

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On 9/28/22 23:40, Thomas Petazzoni wrote:
> On Wed, 28 Sep 2022 15:28:58 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> ./host/bin/aarch64-linux-readelf -h
>> build/linux-custom/tools/perf/tests/pe-file.exe | sed -r -e '/^
>> Machine: +(.+)/!d; s//\1/;' | head -1
>> IMAGE_FILE_MACHINE_AMD64 (0x8664)
> 
> Could you provide the full output? In other words, I'm interested to
> see if it's really readelf showing the same "Machine:" field as for
> regular ELF files, or if it's something somewhat different that we
> could distinguish.

Sure, please find attached.

> 
>> I should mention that the readelf binary in this case is the LLVM Object
>> Reader and it does support PE/COFF which is probably why it even
>> remotely attempts to parse the file.
> 
> How come your readelf is from LLVM? Are you using an external toolchain
> that isn't based on the standard GNU Binutils?

Correct, I am using a custom built LLVM toolchain that uses the Clang 
integrated assembler and none of the binutils binaries, or any GNU 
component for that matter.
-- 
Florian

[-- Attachment #2: pe-file.exe.readelf.output --]
[-- Type: text/plain, Size: 2377 bytes --]

ImageFileHeader {
  Machine: IMAGE_FILE_MACHINE_AMD64 (0x8664)
  SectionCount: 11
  TimeDateStamp: 1970-01-01 00:00:00 (0x0)
  PointerToSymbolTable: 0xC000
  SymbolCount: 1115
  StringTableSize: 6373
  OptionalHeaderSize: 240
  Characteristics [ (0x27)
    IMAGE_FILE_EXECUTABLE_IMAGE (0x2)
    IMAGE_FILE_LARGE_ADDRESS_AWARE (0x20)
    IMAGE_FILE_LINE_NUMS_STRIPPED (0x4)
    IMAGE_FILE_RELOCS_STRIPPED (0x1)
  ]
}
ImageOptionalHeader {
  Magic: 0x20B
  MajorLinkerVersion: 2
  MinorLinkerVersion: 34
  SizeOfCode: 8192
  SizeOfInitializedData: 40960
  SizeOfUninitializedData: 4096
  AddressOfEntryPoint: 0x14F0
  BaseOfCode: 0x1000
  ImageBase: 0x400000
  SectionAlignment: 4096
  FileAlignment: 4096
  MajorOperatingSystemVersion: 4
  MinorOperatingSystemVersion: 0
  MajorImageVersion: 0
  MinorImageVersion: 0
  MajorSubsystemVersion: 5
  MinorSubsystemVersion: 2
  SizeOfImage: 53248
  SizeOfHeaders: 4096
  Subsystem: IMAGE_SUBSYSTEM_WINDOWS_CUI (0x3)
  Characteristics [ (0x0)
  ]
  SizeOfStackReserve: 2097152
  SizeOfStackCommit: 4096
  SizeOfHeapReserve: 1048576
  SizeOfHeapCommit: 4096
  NumberOfRvaAndSize: 16
  DataDirectory {
    ExportTableRVA: 0x0
    ExportTableSize: 0x0
    ImportTableRVA: 0x9000
    ImportTableSize: 0x750
    ResourceTableRVA: 0x0
    ResourceTableSize: 0x0
    ExceptionTableRVA: 0x6000
    ExceptionTableSize: 0x258
    CertificateTableRVA: 0x0
    CertificateTableSize: 0x0
    BaseRelocationTableRVA: 0x0
    BaseRelocationTableSize: 0x0
    DebugRVA: 0x5000
    DebugSize: 0x1C
    ArchitectureRVA: 0x0
    ArchitectureSize: 0x0
    GlobalPtrRVA: 0x0
    GlobalPtrSize: 0x0
    TLSTableRVA: 0x4040
    TLSTableSize: 0x28
    LoadConfigTableRVA: 0x0
    LoadConfigTableSize: 0x0
    BoundImportRVA: 0x0
    BoundImportSize: 0x0
    IATRVA: 0x91CC
    IATSize: 0x190
    DelayImportDescriptorRVA: 0x0
    DelayImportDescriptorSize: 0x0
    CLRRuntimeHeaderRVA: 0x0
    CLRRuntimeHeaderSize: 0x0
    ReservedRVA: 0x0
    ReservedSize: 0x0
  }
}
DOSHeader {
  Magic: MZ
  UsedBytesInTheLastPage: 144
  FileSizeInPages: 3
  NumberOfRelocationItems: 0
  HeaderSizeInParagraphs: 4
  MinimumExtraParagraphs: 0
  MaximumExtraParagraphs: 65535
  InitialRelativeSS: 0
  InitialSP: 184
  Checksum: 0
  InitialIP: 0
  InitialRelativeCS: 0
  AddressOfRelocationTable: 64
  OverlayNumber: 0
  OEMid: 0
  OEMinfo: 0
  AddressOfNewExeHeader: 128
}

[-- Attachment #3: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-28 22:28   ` Florian Fainelli
  2022-09-29  6:40     ` Thomas Petazzoni
@ 2022-10-01 19:07     ` Yann E. MORIN
  2022-10-03 17:21       ` Florian Fainelli
  1 sibling, 1 reply; 11+ messages in thread
From: Yann E. MORIN @ 2022-10-01 19:07 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Thomas Petazzoni, buildroot

Florian, All,

On 2022-09-28 15:28 -0700, Florian Fainelli spake thusly:
> On 9/28/22 14:38, Thomas Petazzoni wrote:
> >On Tue, 27 Sep 2022 15:11:33 -0700
> >Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
> >>("perf tests: Add test for PE binary format support") present in >=
> >>v5.10 there is an unconditional installation of PE binaries which will
> >>be rejected by the check-bin-arch script.
> >>
> >>Make sure that these binaries are excluded from being checked to allow
> >>the installation of the perf tests.
> >>
> >>Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
> >>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >
> >Thanks for the patch! Before merging it, I'd like to understand a
> >little bit more how readelf behaves for these PE files. Indeed in
> >check-bin-arch, we are doing:
> >
> >         arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
> >                        sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)
> >
> >         # If no architecture found, assume it was not an ELF file
> >         if test "${arch}" = "" ; then
> >                 continue
> >         fi
> >
> >for a PE file, I would expect readelf to badly fail, and therefore
> >${arch} to be empty, and the file simply ignored.
> >
> >What is the behavior/output of readelf on these PE files?
> 
> If I use my host system readelf which is packaged from
> binutils-x86-64-linux-gnu, I get no output and all is well, however when
> check-bin-arch is called and it uses $(TARGET_READELF), I do get:
> 
> ./host/bin/aarch64-linux-readelf -h
> build/linux-custom/tools/perf/tests/pe-file.exe | sed -r -e '/^ Machine:
> +(.+)/!d; s//\1/;' | head -1
> IMAGE_FILE_MACHINE_AMD64 (0x8664)
> 
> which is what prompted me to issue this patch in the first place.
> 
> I should mention that the readelf binary in this case is the LLVM Object
> Reader and it does support PE/COFF which is probably why it even remotely
> attempts to parse the file.

OK, but isn't that flawed? It is named 'readelf', so I would expect it
to not report on non-ELF files, at least by default (i.e. unless a
specific option flag is passed).

If a readelf tool does not behave like a readelf tool should behave,
isn't that a bug in that specific readelf tool to begin with?

Otherwise, it is very intriguing how you managed to have a toolchain
without gcc et al, be usable for Buildroot that only knows about the GNU
ecosystem.

Is that a publicly available toolchain? Can you share the recipe how
that toolchain was generated? Do you have special, local changes in
Buildroot to accomodate that toolchain? Will you submit those changes?

That would allow adding more testing to avoid such issue in the future.
Otherwise, I am not fan of adding such exclusion for private cases that
we can not (easily) reproduce, as those cases are not supported.

Regards,
Yann E. MORIN.

> -- 
> Florian
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-10-01 19:07     ` Yann E. MORIN
@ 2022-10-03 17:21       ` Florian Fainelli
  2022-10-03 19:15         ` Yann E. MORIN
  0 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2022-10-03 17:21 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: Thomas Petazzoni, buildroot

On 10/1/22 12:07, Yann E. MORIN wrote:
> Florian, All,
> 
> On 2022-09-28 15:28 -0700, Florian Fainelli spake thusly:
>> On 9/28/22 14:38, Thomas Petazzoni wrote:
>>> On Tue, 27 Sep 2022 15:11:33 -0700
>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>> Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
>>>> ("perf tests: Add test for PE binary format support") present in >=
>>>> v5.10 there is an unconditional installation of PE binaries which will
>>>> be rejected by the check-bin-arch script.
>>>>
>>>> Make sure that these binaries are excluded from being checked to allow
>>>> the installation of the perf tests.
>>>>
>>>> Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>
>>> Thanks for the patch! Before merging it, I'd like to understand a
>>> little bit more how readelf behaves for these PE files. Indeed in
>>> check-bin-arch, we are doing:
>>>
>>>          arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
>>>                         sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)
>>>
>>>          # If no architecture found, assume it was not an ELF file
>>>          if test "${arch}" = "" ; then
>>>                  continue
>>>          fi
>>>
>>> for a PE file, I would expect readelf to badly fail, and therefore
>>> ${arch} to be empty, and the file simply ignored.
>>>
>>> What is the behavior/output of readelf on these PE files?
>>
>> If I use my host system readelf which is packaged from
>> binutils-x86-64-linux-gnu, I get no output and all is well, however when
>> check-bin-arch is called and it uses $(TARGET_READELF), I do get:
>>
>> ./host/bin/aarch64-linux-readelf -h
>> build/linux-custom/tools/perf/tests/pe-file.exe | sed -r -e '/^ Machine:
>> +(.+)/!d; s//\1/;' | head -1
>> IMAGE_FILE_MACHINE_AMD64 (0x8664)
>>
>> which is what prompted me to issue this patch in the first place.
>>
>> I should mention that the readelf binary in this case is the LLVM Object
>> Reader and it does support PE/COFF which is probably why it even remotely
>> attempts to parse the file.
> 
> OK, but isn't that flawed? It is named 'readelf', so I would expect it
> to not report on non-ELF files, at least by default (i.e. unless a
> specific option flag is passed).
> 
> If a readelf tool does not behave like a readelf tool should behave,
> isn't that a bug in that specific readelf tool to begin with?

Markus can correct me but it seems to me llvm-readelf is LLVM's own 
doing and aliases to llvm-readobj, given that PE executable support was 
built into the toolchain, it happily went into parsing the PE files. I 
suppose that in premise llvm-readelf should retrict itself to parsing 
ELF files, clearly LLVM developers thought differently.

> 
> Otherwise, it is very intriguing how you managed to have a toolchain
> without gcc et al, be usable for Buildroot that only knows about the GNU
> ecosystem.
> 
> Is that a publicly available toolchain? 

Not yet, but soon.

> Can you share the recipe how that toolchain was generated?

Yes that will be done.

> Do you have special, local changes in Buildroot to accomodate that toolchain? Will you submit those changes?

Yes a few and yes we will, provided there is interest from the buildroot 
community to have support for a GNU-free and musl-libc based toolchain. 
The approach we took for now is that we provide a GNU-compatible wrapper 
and set of executable names such that you build with clang/LLVM but you 
don't really need to know about it, except where it matters.

> 
> That would allow adding more testing to avoid such issue in the future.
> Otherwise, I am not fan of adding such exclusion for private cases that
> we can not (easily) reproduce, as those cases are not supported.

Fair enough, how about we make sure the toolchain is available, can be 
re-created by anyone who desires so and then we submit the changes to 
buildroot that we have so you see the full picture?
-- 
Florian
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-10-03 17:21       ` Florian Fainelli
@ 2022-10-03 19:15         ` Yann E. MORIN
  0 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2022-10-03 19:15 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Thomas Petazzoni, buildroot

Florian, All,

On 2022-10-03 10:21 -0700, Florian Fainelli spake thusly:
> On 10/1/22 12:07, Yann E. MORIN wrote:
> >On 2022-09-28 15:28 -0700, Florian Fainelli spake thusly:
> >>On 9/28/22 14:38, Thomas Petazzoni wrote:
> >>>On Tue, 27 Sep 2022 15:11:33 -0700
> >>>Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
> >>>>("perf tests: Add test for PE binary format support") present in >=
> >>>>v5.10 there is an unconditional installation of PE binaries which will
> >>>>be rejected by the check-bin-arch script.
> >>>>
> >>>>Make sure that these binaries are excluded from being checked to allow
> >>>>the installation of the perf tests.
[--SNIP--]
> >>I should mention that the readelf binary in this case is the LLVM Object
> >>Reader and it does support PE/COFF which is probably why it even remotely
> >>attempts to parse the file.
> >
> >Is that a publicly available toolchain?
> >Can you share the recipe how that toolchain was generated?
> Yes that will be done.

Great!

> >Do you have special, local changes in Buildroot to accomodate that toolchain? Will you submit those changes?
> Yes a few and yes we will, provided there is interest from the buildroot
> community to have support for a GNU-free and musl-libc based toolchain.

I am personally not interested in a "GNU-free" toolchain. However, I am
very much interested in an llvm-based toolchain.

Semantics, semantics! ;-)

> The
> approach we took for now is that we provide a GNU-compatible wrapper and set
> of executable names such that you build with clang/LLVM but you don't really
> need to know about it, except where it matters.

Yes, that sounds like a good approach.

> >That would allow adding more testing to avoid such issue in the future.
> >Otherwise, I am not fan of adding such exclusion for private cases that
> >we can not (easily) reproduce, as those cases are not supported.
> Fair enough, how about we make sure the toolchain is available, can be
> re-created by anyone who desires so and then we submit the changes to
> buildroot that we have so you see the full picture?

Yes, that would be awesome!

As I said above: having support for using an llvm-based toolchain would
be very interesting!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-27 22:11 [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test Florian Fainelli
  2022-09-28 21:38 ` Thomas Petazzoni
@ 2023-04-17 19:58 ` Yann E. MORIN
  2023-04-19  0:27   ` Florian Fainelli
  2023-04-23 10:40 ` Peter Korsgaard
  2 siblings, 1 reply; 11+ messages in thread
From: Yann E. MORIN @ 2023-04-17 19:58 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: buildroot

Florian, All,

On 2022-09-27 15:11 -0700, Florian Fainelli spake thusly:
> Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
> ("perf tests: Add test for PE binary format support") present in >=
> v5.10 there is an unconditional installation of PE binaries which will
> be rejected by the check-bin-arch script.
> 
> Make sure that these binaries are excluded from being checked to allow
> the installation of the perf tests.
> 
> Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

I've finally applied to master, thanks.

However, I wonder if we could not be a bit more robust in check-bin-arch
itself, something like:

    diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
    index 27cc59bca0..2920a95ae3 100755
    --- a/support/scripts/check-bin-arch
    +++ b/support/scripts/check-bin-arch
    @@ -58,6 +58,9 @@ exitcode=0
     IFS="
     "
     
    +# ELF magic
    +ELF_MAGIC="$(printf "\x7f\x45\x4c\x46")"
    +
     while read f; do
     	for ignore in "${IGNORES[@]}"; do
     		if [[ "${f}" =~ ^"${ignore}" ]]; then
    @@ -71,13 +74,19 @@ while read f; do
     		continue
     	fi
     
    +	# Check if it looks like it is an ELF file
    +	read -N 4 magic <"${TARGET_DIR}/${f}"
    +	if [ "${magic}" != "${ELF_MAGIC}" ]; then
    +		continue
    +	fi
    +
     	# Get architecture using readelf. We pipe through 'head -1' so
     	# that when the file is a static library (.a), we only take
     	# into account the architecture of the first object file.
     	arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
     		       sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)
     
    -# If no architecture found, assume it was not an ELF file
    +# If no architecture found, assume it was in fact not an ELF file
     	if test "${arch}" = "" ; then
     		continue
     	fi

(with the read -N 4 trick as discussed in [0])

Thoughts?

[0] https://lore.kernel.org/buildroot/10267_1663749711_632ACE4F_10267_83_8_20220921084149.GB2398274@tl-lnx-nyma7486/

Regards,
Yann E. MORIN.

> ---
>  package/linux-tools/linux-tool-perf.mk.in | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in
> index dda63cccecb4..c22097316264 100644
> --- a/package/linux-tools/linux-tool-perf.mk.in
> +++ b/package/linux-tools/linux-tool-perf.mk.in
> @@ -169,6 +169,10 @@ define PERF_INSTALL_REMOVE_SCRIPTS
>  	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/scripts/
>  	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/tests/
>  endef
> +
> +LINUX_TOOLS_BIN_ARCH_EXCLUDE += \
> +	/usr/libexec/perf-core/tests/pe-file.exe \
> +	/usr/libexec/perf-core/tests/pe-file.exe.debug
>  endif
>  
>  define PERF_INSTALL_TARGET_CMDS
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2023-04-17 19:58 ` Yann E. MORIN
@ 2023-04-19  0:27   ` Florian Fainelli
  0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2023-04-19  0:27 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot



On 4/17/2023 12:58 PM, Yann E. MORIN wrote:
> Florian, All,
> 
> On 2022-09-27 15:11 -0700, Florian Fainelli spake thusly:
>> Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
>> ("perf tests: Add test for PE binary format support") present in >=
>> v5.10 there is an unconditional installation of PE binaries which will
>> be rejected by the check-bin-arch script.
>>
>> Make sure that these binaries are excluded from being checked to allow
>> the installation of the perf tests.
>>
>> Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> I've finally applied to master, thanks.
> 
> However, I wonder if we could not be a bit more robust in check-bin-arch
> itself, something like:
> 
>      diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
>      index 27cc59bca0..2920a95ae3 100755
>      --- a/support/scripts/check-bin-arch
>      +++ b/support/scripts/check-bin-arch
>      @@ -58,6 +58,9 @@ exitcode=0
>       IFS="
>       "
>       
>      +# ELF magic
>      +ELF_MAGIC="$(printf "\x7f\x45\x4c\x46")"
>      +
>       while read f; do
>       	for ignore in "${IGNORES[@]}"; do
>       		if [[ "${f}" =~ ^"${ignore}" ]]; then
>      @@ -71,13 +74,19 @@ while read f; do
>       		continue
>       	fi
>       
>      +	# Check if it looks like it is an ELF file
>      +	read -N 4 magic <"${TARGET_DIR}/${f}"
>      +	if [ "${magic}" != "${ELF_MAGIC}" ]; then
>      +		continue
>      +	fi
>      +
>       	# Get architecture using readelf. We pipe through 'head -1' so
>       	# that when the file is a static library (.a), we only take
>       	# into account the architecture of the first object file.
>       	arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
>       		       sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)
>       
>      -# If no architecture found, assume it was not an ELF file
>      +# If no architecture found, assume it was in fact not an ELF file
>       	if test "${arch}" = "" ; then
>       		continue
>       	fi
> 
> (with the read -N 4 trick as discussed in [0])
> 
> Thoughts?

That would work for me!
-- 
Florian
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
  2022-09-27 22:11 [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test Florian Fainelli
  2022-09-28 21:38 ` Thomas Petazzoni
  2023-04-17 19:58 ` Yann E. MORIN
@ 2023-04-23 10:40 ` Peter Korsgaard
  2 siblings, 0 replies; 11+ messages in thread
From: Peter Korsgaard @ 2023-04-23 10:40 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: buildroot

>>>>> "Florian" == Florian Fainelli <f.fainelli@gmail.com> writes:

 > Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
 > ("perf tests: Add test for PE binary format support") present in >=
 > v5.10 there is an unconditional installation of PE binaries which will
 > be rejected by the check-bin-arch script.

 > Make sure that these binaries are excluded from being checked to allow
 > the installation of the perf tests.

 > Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
 > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Committed to 2023.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-23 10:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27 22:11 [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test Florian Fainelli
2022-09-28 21:38 ` Thomas Petazzoni
2022-09-28 22:28   ` Florian Fainelli
2022-09-29  6:40     ` Thomas Petazzoni
2022-09-30 22:05       ` Florian Fainelli
2022-10-01 19:07     ` Yann E. MORIN
2022-10-03 17:21       ` Florian Fainelli
2022-10-03 19:15         ` Yann E. MORIN
2023-04-17 19:58 ` Yann E. MORIN
2023-04-19  0:27   ` Florian Fainelli
2023-04-23 10:40 ` Peter Korsgaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).