Avoid undefined behaviour in m68hc11 md_begin
Checks
Commit Message
Given p = A where p is a pointer to some type and A is an array of
that type, then the expression p - 1 + 1 evokes undefined behaviour
according to the C standard.
gcc-13 -fsanitize=address,undefined complains about this, but not
where the undefined behaviour actually occurs at tc-m68hc11.c:646.
Instead you get an error: "tc-m68hc11.c:708:20: runtime error: store
to address 0x62600000016c with insufficient space for an object of
type 'int'". Which is a lie. There most definitely is space there.
Oh well, diagnostics are sometimes hard to get right. The UB is easy
to avoid.
PR 30279
* config/tc-m68hc11.c (md_begin): Avoid undefined pointer
decrement. Remove unnecessary cast.
Comments
On Mär 28 2023, Alan Modra via Binutils wrote:
> Given p = A where p is a pointer to some type and A is an array of
> that type, then the expression p - 1 + 1 evokes undefined behaviour
> according to the C standard.
>
> gcc-13 -fsanitize=address,undefined complains about this, but not
> where the undefined behaviour actually occurs at tc-m68hc11.c:646.
> Instead you get an error: "tc-m68hc11.c:708:20: runtime error: store
> to address 0x62600000016c with insufficient space for an object of
> type 'int'". Which is a lie.
It's not a lie if you regard p-1 as no longer being associated with A,
so that p-1+1 cannot move back into it any more.
On Tue, Mar 28, 2023 at 01:19:38PM +0200, Andreas Schwab wrote:
> On Mär 28 2023, Alan Modra via Binutils wrote:
>
> > Given p = A where p is a pointer to some type and A is an array of
> > that type, then the expression p - 1 + 1 evokes undefined behaviour
> > according to the C standard.
> >
> > gcc-13 -fsanitize=address,undefined complains about this, but not
> > where the undefined behaviour actually occurs at tc-m68hc11.c:646.
> > Instead you get an error: "tc-m68hc11.c:708:20: runtime error: store
> > to address 0x62600000016c with insufficient space for an object of
> > type 'int'". Which is a lie.
>
> It's not a lie if you regard p-1 as no longer being associated with A,
> so that p-1+1 cannot move back into it any more.
The error message specifies an address. That address is part of the
memory allocated. To say there is insufficient space at that address
is indeed a lie.
Please note that I'm not saying anyone should spend time fixing this
diagnostic. I truly don't care. After all, it was good enough to
find the undefined behaviour in a relatively short time.
On Mär 28 2023, Alan Modra wrote:
> On Tue, Mar 28, 2023 at 01:19:38PM +0200, Andreas Schwab wrote:
>> On Mär 28 2023, Alan Modra via Binutils wrote:
>>
>> > Given p = A where p is a pointer to some type and A is an array of
>> > that type, then the expression p - 1 + 1 evokes undefined behaviour
>> > according to the C standard.
>> >
>> > gcc-13 -fsanitize=address,undefined complains about this, but not
>> > where the undefined behaviour actually occurs at tc-m68hc11.c:646.
>> > Instead you get an error: "tc-m68hc11.c:708:20: runtime error: store
>> > to address 0x62600000016c with insufficient space for an object of
>> > type 'int'". Which is a lie.
>>
>> It's not a lie if you regard p-1 as no longer being associated with A,
>> so that p-1+1 cannot move back into it any more.
>
> The error message specifies an address. That address is part of the
> memory allocated. To say there is insufficient space at that address
> is indeed a lie.
It's the same lie when you fall off the end of an array into a
neighboring array.
On Tue, Mar 28, 2023 at 02:30:10PM +0200, Andreas Schwab wrote:
> On Mär 28 2023, Alan Modra wrote:
>
> > On Tue, Mar 28, 2023 at 01:19:38PM +0200, Andreas Schwab wrote:
> >> On Mär 28 2023, Alan Modra via Binutils wrote:
> >>
> >> > Given p = A where p is a pointer to some type and A is an array of
> >> > that type, then the expression p - 1 + 1 evokes undefined behaviour
> >> > according to the C standard.
> >> >
> >> > gcc-13 -fsanitize=address,undefined complains about this, but not
> >> > where the undefined behaviour actually occurs at tc-m68hc11.c:646.
> >> > Instead you get an error: "tc-m68hc11.c:708:20: runtime error: store
> >> > to address 0x62600000016c with insufficient space for an object of
> >> > type 'int'". Which is a lie.
> >>
> >> It's not a lie if you regard p-1 as no longer being associated with A,
> >> so that p-1+1 cannot move back into it any more.
> >
> > The error message specifies an address. That address is part of the
> > memory allocated. To say there is insufficient space at that address
> > is indeed a lie.
>
> It's the same lie when you fall off the end of an array into a
> neighboring array.
I'm beginning to think you are deliberately provoking me. There is no
falling off the end of an array here. See the testcase in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109308.
On Mär 29 2023, Alan Modra wrote:
> On Tue, Mar 28, 2023 at 02:30:10PM +0200, Andreas Schwab wrote:
>> On Mär 28 2023, Alan Modra wrote:
>>
>> > On Tue, Mar 28, 2023 at 01:19:38PM +0200, Andreas Schwab wrote:
>> >> On Mär 28 2023, Alan Modra via Binutils wrote:
>> >>
>> >> > Given p = A where p is a pointer to some type and A is an array of
>> >> > that type, then the expression p - 1 + 1 evokes undefined behaviour
>> >> > according to the C standard.
>> >> >
>> >> > gcc-13 -fsanitize=address,undefined complains about this, but not
>> >> > where the undefined behaviour actually occurs at tc-m68hc11.c:646.
>> >> > Instead you get an error: "tc-m68hc11.c:708:20: runtime error: store
>> >> > to address 0x62600000016c with insufficient space for an object of
>> >> > type 'int'". Which is a lie.
>> >>
>> >> It's not a lie if you regard p-1 as no longer being associated with A,
>> >> so that p-1+1 cannot move back into it any more.
>> >
>> > The error message specifies an address. That address is part of the
>> > memory allocated. To say there is insufficient space at that address
>> > is indeed a lie.
>>
>> It's the same lie when you fall off the end of an array into a
>> neighboring array.
>
> I'm beginning to think you are deliberately provoking me. There is no
> falling off the end of an array here.
I didn't say that. I was just comparing it to other situations where
there is insufficient space the address even though the memory exists.
Similar to this there is no allocated space at the address p-1, so p-1+1
cannot be part of an allocated space.
@@ -643,7 +643,7 @@ md_begin (void)
(int (*) (const void*, const void*)) cmp_opcode);
opc = XNEWVEC (struct m68hc11_opcode_def, num_opcodes);
- m68hc11_opcode_defs = opc--;
+ m68hc11_opcode_defs = opc;
/* Insert unique names into hash table. The M6811 instruction set
has several identical opcode names that have different opcodes based
@@ -655,19 +655,18 @@ md_begin (void)
if (strcmp (prev_name, opcodes->name))
{
- prev_name = (char *) opcodes->name;
-
+ prev_name = opcodes->name;
opc++;
- opc->format = 0;
- opc->min_operands = 100;
- opc->max_operands = 0;
- opc->nb_modes = 0;
- opc->opcode = opcodes;
- opc->used = 0;
- str_hash_insert (m68hc11_hash, opcodes->name, opc, 0);
+ (opc - 1)->format = 0;
+ (opc - 1)->min_operands = 100;
+ (opc - 1)->max_operands = 0;
+ (opc - 1)->nb_modes = 0;
+ (opc - 1)->opcode = opcodes;
+ (opc - 1)->used = 0;
+ str_hash_insert (m68hc11_hash, opcodes->name, opc - 1, 0);
}
- opc->nb_modes++;
- opc->format |= opcodes->format;
+ (opc - 1)->nb_modes++;
+ (opc - 1)->format |= opcodes->format;
/* See how many operands this opcode needs. */
expect = 0;
@@ -700,14 +699,13 @@ md_begin (void)
expect++;
}
- if (expect < opc->min_operands)
- opc->min_operands = expect;
+ if (expect < (opc - 1)->min_operands)
+ (opc - 1)->min_operands = expect;
if (IS_CALL_SYMBOL (opcodes->format))
expect++;
- if (expect > opc->max_operands)
- opc->max_operands = expect;
+ if (expect > (opc - 1)->max_operands)
+ (opc - 1)->max_operands = expect;
}
- opc++;
m68hc11_nb_opcode_defs = opc - m68hc11_opcode_defs;
if (flag_print_opcodes)