2006-05-05 08:41:34

by Denis Vlasenko

[permalink] [raw]
Subject: as bug (was: Re: smp/up alternatives crash when CONFIG_HOTPLUG_CPU)

[binutils list CC'ed]

On Friday 21 April 2006 10:48, Ingo Molnar wrote:
> you can also try the mutex.bad.s file i attached:
>
> $ as mutex.s.bad
> mutex.s.bad: Assembler messages:
> mutex.s.bad:267: Warning: .space or .fill with negative value, ignored
> mutex.s.bad:355: Warning: .space or .fill with negative value, ignored
> mutex.s.bad:412: Warning: .space or .fill with negative value, ignored
> mutex.s.bad:574: Warning: .space or .fill with negative value, ignored
> mutex.s.bad:627: Warning: .space or .fill with negative value, ignored

Reduced testcase, which still exhibits the bug.

# as mutex.bad_minimal.s
mutex.bad_minimal.s: Assembler messages:
mutex.bad_minimal.s:21: Warning: .space or .fill with negative value, ignored
# as --version | head -1
GNU assembler 2.15.91.0.1 20040527
# cat mutex.bad_minimal.s

661:
662:
.section .smp_altinstructions,"a"
.align 4
.long 661b
.byte 0x68
.byte 662b-661b
.section .smp_altinstr_replacement,"awx"
.fill 662b-661b,1,0x42
.section .sched.text,"ax",@progbits
call _spin_unlock #
661:
2: jle 2b #
662:
.section .smp_altinstructions,"a"
.align 4
.long 661b
.byte 0x68
.byte 662b-661b
.section .smp_altinstr_replacement,"awx"
.fill 662b-661b,1,0x42

--
vda


2006-05-05 08:47:09

by Denis Vlasenko

[permalink] [raw]
Subject: Re: as bug (was: Re: smp/up alternatives crash when CONFIG_HOTPLUG_CPU)

On Friday 05 May 2006 11:40, Denis Vlasenko wrote:
> Reduced testcase, which still exhibits the bug.

Smaller one:

.section .smp_altinstr_replacement,"awx"
.section .sched.text,"ax",@progbits
call _spin_unlock #
661:
2: jle 2b #
662:
.section .smp_altinstr_replacement,"awx"
.fill 662b-661b,1,0x42

--
vda

2006-05-05 12:20:33

by Alan Modra

[permalink] [raw]
Subject: Re: as bug (was: Re: smp/up alternatives crash when CONFIG_HOTPLUG_CPU)

On Fri, May 05, 2006 at 11:45:54AM +0300, Denis Vlasenko wrote:
>
> .section .smp_altinstr_replacement,"awx"
> .section .sched.text,"ax",@progbits
> call _spin_unlock #
> 661:
> 2: jle 2b #
> 662:
> .section .smp_altinstr_replacement,"awx"
> .fill 662b-661b,1,0x42

gas should give a better error message here, but really, gas shouldn't
be expected to assemble this. In essence, you have forward references
in that expression for the .fill length..

Some background: Gas is a single pass assembler. It emits code and
data into "frags", buffers containing some fixed number of bytes and
possibly a variable length tail. The variable length part allows
various features, notably that of variable length instructions. Symbols
are defined relative to their frags. Until the frag addresses are
finalized, an expresion involving subtraction of two symbols in
different frags cannot be evaluated correctly. With the testcase above
you have exactly that situation. The x86 "jle" instruction can be two
sizes, either 6 bytes or 2 bytes depending on the offset needed, and gas
doesn't have the smarts to recognize that the "jle" above is just 2
bytes. Instead, it assumes a variable size, putting the "jle" in its
own frag. This means that label "661" and "662" are in separate frags
with "661" at offset 5 in its frag, and "662" at offset 0.

Since you define the ".smp_altinstr_replacement" section before the
".sched.text section", gas tries to finalize ".smp_altinstr_replacement"
first. When it tries to calculate the fill size using
(<base addr "662" frag>+<offset "662">)
- (<base addr "661" frag>+<offset "661">)
the frag base addresses have not yet been set, and zero is used. ie.
gas tries to assemble ".fill -5,1,0x42".

A workaround is to ensure that the ".sched.text" section is defined
before ".smp_altinst_replacement".

--
Alan Modra
IBM OzLabs - Linux Technology Centre

2006-05-05 13:13:51

by Denis Vlasenko

[permalink] [raw]
Subject: Re: as bug (was: Re: smp/up alternatives crash when CONFIG_HOTPLUG_CPU)

On Friday 05 May 2006 15:20, Alan Modra wrote:
> On Fri, May 05, 2006 at 11:45:54AM +0300, Denis Vlasenko wrote:
> >
> > .section .smp_altinstr_replacement,"awx"
> > .section .sched.text,"ax",@progbits
> > call _spin_unlock #
> > 661:
> > 2: jle 2b #
> > 662:
> > .section .smp_altinstr_replacement,"awx"
> > .fill 662b-661b,1,0x42
>
> gas should give a better error message here, but really, gas shouldn't
> be expected to assemble this. In essence, you have forward references
> in that expression for the .fill length..

Yes, this should be an error, not warning. It produces miscompiled
object modules.

> Some background: Gas is a single pass assembler. It emits code and
> data into "frags", buffers containing some fixed number of bytes and
> possibly a variable length tail. The variable length part allows
> various features, notably that of variable length instructions. Symbols
> are defined relative to their frags. Until the frag addresses are
> finalized, an expresion involving subtraction of two symbols in
> different frags cannot be evaluated correctly. With the testcase above
> you have exactly that situation. The x86 "jle" instruction can be two
> sizes, either 6 bytes or 2 bytes depending on the offset needed, and gas
> doesn't have the smarts to recognize that the "jle" above is just 2
> bytes. Instead, it assumes a variable size, putting the "jle" in its
> own frag. This means that label "661" and "662" are in separate frags
> with "661" at offset 5 in its frag, and "662" at offset 0.
>
> Since you define the ".smp_altinstr_replacement" section before the
> ".sched.text section", gas tries to finalize ".smp_altinstr_replacement"
> first. When it tries to calculate the fill size using
> (<base addr "662" frag>+<offset "662">)
> - (<base addr "661" frag>+<offset "661">)
> the frag base addresses have not yet been set, and zero is used. ie.
> gas tries to assemble ".fill -5,1,0x42".

Thanks for the explanation.
--
vda

2006-05-06 03:11:43

by Alan Modra

[permalink] [raw]
Subject: Re: as bug (was: Re: smp/up alternatives crash when CONFIG_HOTPLUG_CPU)

On Fri, May 05, 2006 at 04:13:24PM +0300, Denis Vlasenko wrote:
> On Friday 05 May 2006 15:20, Alan Modra wrote:
> > the frag base addresses have not yet been set, and zero is used. ie.
> > gas tries to assemble ".fill -5,1,0x42".

The fact that enabling gas listings fixes this has been nagging at me
since writing the sketchy description of gas frags and relaxation.
I'd forgotten that relaxation keeps iterating over all sections until no
frag changes address. ie. even though the first .fill is using invalid
addresses, there will be a subsequent pass that uses the correct value.

The reason why gas -al helps with this case is that gas creates a new
frag for each line as somewhere to hang the file/line number info. So
both "661" and "662" start off at offset zero in their frags and the
initial pass .fill has a zero length rather than a negative one.

So perhaps gas ought to be able to handle this after all.. I'll see if
I can come up with a fix.

--
Alan Modra
IBM OzLabs - Linux Technology Centre