[bpf-next] selftests/bpf: fix build-id for liburandom_read.so

Message ID 20221104094016.102049-1-asavkov@redhat.com
State New
Headers
Series [bpf-next] selftests/bpf: fix build-id for liburandom_read.so |

Commit Message

Artem Savkov Nov. 4, 2022, 9:40 a.m. UTC
  lld produces "fast" style build-ids by default, which is inconsistent
with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
when linking liburandom_read.so the same way it is already done for
urandom_read.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 tools/testing/selftests/bpf/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

KP Singh Nov. 4, 2022, 1:29 p.m. UTC | #1
On Fri, Nov 4, 2022 at 10:41 AM Artem Savkov <asavkov@redhat.com> wrote:
>
> lld produces "fast" style build-ids by default, which is inconsistent
> with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
> when linking liburandom_read.so the same way it is already done for
> urandom_read.
>
> Signed-off-by: Artem Savkov <asavkov@redhat.com>

Acked-by: KP Singh <kpsingh@kernel.org>

This was done in
https://lore.kernel.org/bpf/20200922232140.1994390-1-morbo@google.com
  
Daniel Borkmann Nov. 4, 2022, 5:38 p.m. UTC | #2
Hi Artem,

On 11/4/22 2:29 PM, KP Singh wrote:
> On Fri, Nov 4, 2022 at 10:41 AM Artem Savkov <asavkov@redhat.com> wrote:
>>
>> lld produces "fast" style build-ids by default, which is inconsistent
>> with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
>> when linking liburandom_read.so the same way it is already done for
>> urandom_read.
>>
>> Signed-off-by: Artem Savkov <asavkov@redhat.com>
> 
> Acked-by: KP Singh <kpsingh@kernel.org>
> 
> This was done in
> https://lore.kernel.org/bpf/20200922232140.1994390-1-morbo@google.com

When you say "fix", does it actually fix a failing test case or is it more
of a cleanup to align liburandom_read build with urandom_read? From glancing
at the code, we only check build id for urandom_read.

Cheers,
Daniel
  
Andrii Nakryiko Nov. 4, 2022, 10:58 p.m. UTC | #3
On Fri, Nov 4, 2022 at 10:38 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Hi Artem,
>
> On 11/4/22 2:29 PM, KP Singh wrote:
> > On Fri, Nov 4, 2022 at 10:41 AM Artem Savkov <asavkov@redhat.com> wrote:
> >>
> >> lld produces "fast" style build-ids by default, which is inconsistent
> >> with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
> >> when linking liburandom_read.so the same way it is already done for
> >> urandom_read.
> >>
> >> Signed-off-by: Artem Savkov <asavkov@redhat.com>
> >
> > Acked-by: KP Singh <kpsingh@kernel.org>
> >
> > This was done in
> > https://lore.kernel.org/bpf/20200922232140.1994390-1-morbo@google.com
>
> When you say "fix", does it actually fix a failing test case or is it more
> of a cleanup to align liburandom_read build with urandom_read? From glancing
> at the code, we only check build id for urandom_read.

I reworded the subject to "selftests/bpf: Use consistent build-id type
for liburandom_read.so" and pushed. Thanks!

>
> Cheers,
> Daniel
  
patchwork-bot+netdevbpf@kernel.org Nov. 4, 2022, 11 p.m. UTC | #4
Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri,  4 Nov 2022 10:40:16 +0100 you wrote:
> lld produces "fast" style build-ids by default, which is inconsistent
> with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
> when linking liburandom_read.so the same way it is already done for
> urandom_read.
> 
> Signed-off-by: Artem Savkov <asavkov@redhat.com>
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: fix build-id for liburandom_read.so
    https://git.kernel.org/bpf/bpf-next/c/61fc5e66f755

You are awesome, thank you!
  
Daniel Borkmann Nov. 4, 2022, 11:07 p.m. UTC | #5
On 11/4/22 11:58 PM, Andrii Nakryiko wrote:
> On Fri, Nov 4, 2022 at 10:38 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> Hi Artem,
>>
>> On 11/4/22 2:29 PM, KP Singh wrote:
>>> On Fri, Nov 4, 2022 at 10:41 AM Artem Savkov <asavkov@redhat.com> wrote:
>>>>
>>>> lld produces "fast" style build-ids by default, which is inconsistent
>>>> with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
>>>> when linking liburandom_read.so the same way it is already done for
>>>> urandom_read.
>>>>
>>>> Signed-off-by: Artem Savkov <asavkov@redhat.com>
>>>
>>> Acked-by: KP Singh <kpsingh@kernel.org>
>>>
>>> This was done in
>>> https://lore.kernel.org/bpf/20200922232140.1994390-1-morbo@google.com
>>
>> When you say "fix", does it actually fix a failing test case or is it more
>> of a cleanup to align liburandom_read build with urandom_read? From glancing
>> at the code, we only check build id for urandom_read.
> 
> I reworded the subject to "selftests/bpf: Use consistent build-id type
> for liburandom_read.so" and pushed. Thanks!

Ack, sgtm!
  
Artem Savkov Nov. 7, 2022, 7:37 a.m. UTC | #6
On Fri, Nov 04, 2022 at 03:58:43PM -0700, Andrii Nakryiko wrote:
> On Fri, Nov 4, 2022 at 10:38 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > Hi Artem,
> >
> > On 11/4/22 2:29 PM, KP Singh wrote:
> > > On Fri, Nov 4, 2022 at 10:41 AM Artem Savkov <asavkov@redhat.com> wrote:
> > >>
> > >> lld produces "fast" style build-ids by default, which is inconsistent
> > >> with ld's "sha1" style. Explicitly specify build-id style to be "sha1"
> > >> when linking liburandom_read.so the same way it is already done for
> > >> urandom_read.
> > >>
> > >> Signed-off-by: Artem Savkov <asavkov@redhat.com>
> > >
> > > Acked-by: KP Singh <kpsingh@kernel.org>
> > >
> > > This was done in
> > > https://lore.kernel.org/bpf/20200922232140.1994390-1-morbo@google.com
> >
> > When you say "fix", does it actually fix a failing test case or is it more
> > of a cleanup to align liburandom_read build with urandom_read? From glancing
> > at the code, we only check build id for urandom_read.

I called it a "fix" because it broke expectations of external tools, but
the reworded version sounds much better.

> I reworded the subject to "selftests/bpf: Use consistent build-id type
> for liburandom_read.so" and pushed. Thanks!

Thank you, Andrii.
  

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 79edef1dbda4..5a792987df66 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -182,7 +182,8 @@  endif
 $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
 	$(call msg,LIB,,$@)
 	$(Q)$(CLANG) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^ $(LDLIBS)   \
-		     -fuse-ld=$(LLD) -Wl,-znoseparate-code -fPIC -shared -o $@
+		     -fuse-ld=$(LLD) -Wl,-znoseparate-code -Wl,--build-id=sha1 \
+		     -fPIC -shared -o $@
 
 $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
 	$(call msg,BINARY,,$@)