Re: [PATCH] riscv: vdso: fix section overlapping under some conditions

From: Jisheng Zhang <jszhang@kernel.org>
To: Andrew Jones <ajones@ventanamicro.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] riscv: vdso: fix section overlapping under some conditions
Date: Thu, 3 Nov 2022 01:28:11 +0800	[thread overview]
Message-ID: <Y2Koq5QELaoBe7nj@xhacker> (raw)
In-Reply-To: <20221101091942.k7pgcbbkudgjk4ae@kamzik>

On Tue, Nov 01, 2022 at 10:19:42AM +0100, Andrew Jones wrote:
> On Tue, Nov 01, 2022 at 01:58:42AM +0800, Jisheng Zhang wrote:
> > lkp reported a build error, I tried the config and can reproduce
> > build error as below:
> > 
> >   VDSOLD  arch/riscv/kernel/vdso/vdso.so.dbg
> > ld.lld: error: section .note file range overlaps with .text
> > >>> .note range is [0x7C8, 0x803]
> > >>> .text range is [0x800, 0x1993]
> > 
> > ld.lld: error: section .text file range overlaps with .dynamic
> > >>> .text range is [0x800, 0x1993]
> > >>> .dynamic range is [0x808, 0x937]
> > 
> > ld.lld: error: section .note virtual address range overlaps with .text
> > >>> .note range is [0x7C8, 0x803]
> > >>> .text range is [0x800, 0x1993]
> > 
> > Fix it by removing the hardcoding 0x800 and related comments.
> > 
> > Link: https://lore.kernel.org/lkml/202210122123.Cc4FPShJ-lkp@intel.com/#r
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  arch/riscv/kernel/vdso/vdso.lds.S | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> > index 01d94aae5bf5..344209d2e128 100644
> > --- a/arch/riscv/kernel/vdso/vdso.lds.S
> > +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> > @@ -31,13 +31,7 @@ SECTIONS
> >  
> >  	.rodata		: { *(.rodata .rodata.* .gnu.linkonce.r.*) }
> >  
> > -	/*
> > -	 * This linker script is used both with -r and with -shared.
> > -	 * For the layouts to match, we need to skip more than enough
> > -	 * space for the dynamic symbol table, etc. If this amount is
> > -	 * insufficient, ld -shared will error; simply increase it here.
> > -	 */
> > -	. = 0x800;
> 
> Hi Jisheng,

Hi Andrew,

> 
> Removing this hard coded value is a good thing, but I don't understand
> why, if it was necessary before, that it's no longer necessary. Can you
> please explain that in the commit message? If the linker improved in

To be honest, I dunno. The hardcoded 0x800 was there from day1, maybe
Palmer knew the details.

> this regard, then do we need to document a new minimum linker version?

> 
> > +	. = ALIGN(16);
> 
> Aligning text to a 4-byte boundary makes sense to me, but I don't

Aha, I think align text to 4byte is fine. In my old memories, I was
told to align function entry at 16byte boundary, I'm not sure this
is still true.

PS: I just sent out v2 of this fix. The fix method is different and
think v2 is the correct fix while this v1 is an improvement.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv