The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
section in module linker script, which caused symbol lookup tools
like gdb to misinterpret symbol addresses e.g., __ib_process_cq
incorrectly mapping to unrelated functions like below.

  (gdb) disas __ib_process_cq
  Dump of assembler code for function trace_event_fields_cq_schedule:

Removing the empty section restores proper symbol resolution and
layout, ensuring .data placement behaves as expected.

Fixes: 0db6f8d7820a ("alloc_tag: load module tags into separate contiguous memory")
       22d407b164ff ("lib: add allocation tagging support for memory allocation profiling")
Signed-off-by: Casey Chen <cachen@purestorage.com>
Reviewed-by: Yuanyuan Zhong <yzhong@purestorage.com>
Acked-by: Suren Baghdasaryan <surenb@google.com>
---
 include/asm-generic/codetag.lds.h | 6 ------
 scripts/module.lds.S              | 5 -----
 2 files changed, 11 deletions(-)

diff --git a/include/asm-generic/codetag.lds.h b/include/asm-generic/codetag.lds.h
index 372c320c5043..a45fe3d141a1 100644
--- a/include/asm-generic/codetag.lds.h
+++ b/include/asm-generic/codetag.lds.h
@@ -11,12 +11,6 @@
 #define CODETAG_SECTIONS()		\
 	SECTION_WITH_BOUNDARIES(alloc_tags)
 
-/*
- * Module codetags which aren't used after module unload, therefore have the
- * same lifespan as the module and can be safely unloaded with the module.
- */
-#define MOD_CODETAG_SECTIONS()
-
 #define MOD_SEPARATE_CODETAG_SECTION(_name)	\
 	.codetag.##_name : {			\
 		SECTION_WITH_BOUNDARIES(_name)	\
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 711c6e029936..c071ca4beedd 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -50,17 +50,12 @@ SECTIONS {
 	.data : {
 		*(.data .data.[0-9a-zA-Z_]*)
 		*(.data..L*)
-		MOD_CODETAG_SECTIONS()
 	}
 
 	.rodata : {
 		*(.rodata .rodata.[0-9a-zA-Z_]*)
 		*(.rodata..L*)
 	}
-#else
-	.data : {
-		MOD_CODETAG_SECTIONS()
-	}
 #endif
 	MOD_SEPARATE_CODETAG_SECTIONS()
 }
-- 
2.34.1

On 6/10/25 6:22 PM, Casey Chen wrote:
> The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
> section in module linker script, which caused symbol lookup tools
> like gdb to misinterpret symbol addresses e.g., __ib_process_cq
> incorrectly mapping to unrelated functions like below.
> 
>   (gdb) disas __ib_process_cq
>   Dump of assembler code for function trace_event_fields_cq_schedule:
> 
> Removing the empty section restores proper symbol resolution and
> layout, ensuring .data placement behaves as expected.

The patch looks ok me, but I'm somewhat confused about the problem.
I think a linker should not add an empty output section if it doesn't
contain anything, or if .data actually contains something then the extra
dummy definition should be also harmless?

This also reminds me of my previous related fix "codetag: Avoid unused
alloc_tags sections/symbols" [1] which fell through the cracks. I can
rebase it on top of this patch.

[1] https://lore.kernel.org/all/20250313143002.9118-1-petr.pavlu@suse.com/

-- 
Thanks,
Petr

On Tue, Jun 17, 2025 at 2:27 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 6/10/25 6:22 PM, Casey Chen wrote:
> > The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
> > section in module linker script, which caused symbol lookup tools
> > like gdb to misinterpret symbol addresses e.g., __ib_process_cq
> > incorrectly mapping to unrelated functions like below.
> >
> >   (gdb) disas __ib_process_cq
> >   Dump of assembler code for function trace_event_fields_cq_schedule:
> >
> > Removing the empty section restores proper symbol resolution and
> > layout, ensuring .data placement behaves as expected.
>
> The patch looks ok me, but I'm somewhat confused about the problem.
> I think a linker should not add an empty output section if it doesn't
> contain anything, or if .data actually contains something then the extra
> dummy definition should be also harmless?

I also assumed so but apparently this is not entirely harmless.

>
> This also reminds me of my previous related fix "codetag: Avoid unused
> alloc_tags sections/symbols" [1] which fell through the cracks. I can
> rebase it on top of this patch.
>
> [1] https://lore.kernel.org/all/20250313143002.9118-1-petr.pavlu@suse.com/

Yes please.

>
> --
> Thanks,
> Petr