Bug 81720 - [arm] Invalid code generation
Summary: [arm] Invalid code generation
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-04 15:52 UTC by Orlando Arias
Modified: 2017-08-04 17:42 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Testcase to generate the bug. (87 bytes, text/plain)
2017-08-04 15:52 UTC, Orlando Arias
Details
Compiler output with erroneous assembly listing. (353 bytes, text/plain)
2017-08-04 15:53 UTC, Orlando Arias
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Orlando Arias 2017-08-04 15:52:15 UTC
Created attachment 41928 [details]
Testcase to generate the bug.

Greetings,

Yesterday, I ran into a problem with the compiler issuing invalid instructions when compiling code with optimization levels of -O2 or higher. I narrowed it down to the following testcase [attached]:

int main(void) {
	volatile unsigned int read_value = *((volatile unsigned int*)0x00000000);
	return 0;
}

When compiled with

$ arm-none-eabi-gcc -S -c testcase.c -O1 -mthumb -mno-thumb-interwork -mcpu=cortex-m4

The following assembly is generated [trimmed, see full attachment]:

main:
	@ Volatile: function does not return.
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	movs	r3, #0
	ldr	r3, [r3]
	.inst	0xdeff
	.size	main, .-main

The .inst 0xdeff should definitely not be there. Any code that happens after the dereference takes place is never emitted by the compiler. The dereference in question is actually valid for the target I am working with [bare-metal ARM system]. Of course, the assembler happily picks this up and creates object files with the erroneous code.

I am using the following gcc:

$ arm-none-eabi-gcc --version
arm-none-eabi-gcc (Arch Repository) 7.1.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Which has been compiled as:
$ arm-none-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-none-eabi/7.1.0/lto-wrapper
Target: arm-none-eabi
Configured with: /build/arm-none-eabi-gcc/src/gcc-7-20170504/configure --target=arm-none-eabi --prefix=/usr --with-sysroot=/usr/arm-none-eabi --with-native-system-header-dir=/include --libexecdir=/usr/lib --enable-languages=c,c++ --enable-plugins --disable-decimal-float --disable-libffi --disable-libgomp --disable-libmudflap --disable-libquadmath --disable-libssp --disable-libstdcxx-pch --disable-nls --disable-shared --disable-threads --disable-tls --with-gnu-as --with-gnu-ld --with-system-zlib --with-newlib --with-headers=/usr/arm-none-eabi/include --with-python-dir=share/gcc-arm-none-eabi --with-gmp --with-mpfr --with-mpc --with-isl --with-libelf --enable-gnu-indirect-function --with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm' --with-pkgversion='Arch Repository' --with-bugurl=https://bugs.archlinux.org/ --with-multilib-list=armv6-m,armv7-m,armv7e-m,armv7-r
Thread model: single
gcc version 7.1.0 (Arch Repository) 


Any advice? Thank you.

Cheers,
Orlando.
Comment 1 Orlando Arias 2017-08-04 15:53:14 UTC
Created attachment 41929 [details]
Compiler output with erroneous assembly listing.
Comment 2 Andrew Pinski 2017-08-04 16:00:44 UTC
Use -fno-delete-null-pointer-checks as this is a null pointer and deferencing null pointers are undefined in C.
Comment 3 Orlando Arias 2017-08-04 16:18:50 UTC
(In reply to Andrew Pinski from comment #2)
> Use -fno-delete-null-pointer-checks as this is a null pointer and
> deferencing null pointers are undefined in C.

Greetings,

Except that for bare metal targets, address 0 can actually point to a valid object. As such, the dereference is legal by the C standard itself. What you are requesting your users to do here is to issue a compiler flag so that code that they expect to work one way actually performs the way they expect it to. Furthermore, this particular piece of code worked as expected in previous versions of the compiler. 

As such, instead of closing this as invalid, my suggestion is to actually disable this option by default on bare metal targets [which GCC has a quite a few of, by the way]. Thank you.

Cheers,
Orlando.
Comment 4 Andrew Pinski 2017-08-04 16:20:32 UTC
You can have -fno-delete-null-pointer-checks in a specs file which disables it by default for your target ....

Again not a bug.
Comment 5 Richard Earnshaw 2017-08-04 16:24:51 UTC
Bare metal target compilers should still respect the standard, which is quite clear that dereferencing NULL is undefined.  Some users of the compiler expect this behaviour.  It's not about to change.

You have an option you can use, so use it.
Comment 6 Orlando Arias 2017-08-04 17:42:53 UTC
(In reply to Richard Earnshaw from comment #5)
> Bare metal target compilers should still respect the standard, which is
> quite clear that dereferencing NULL is undefined.  Some users of the
> compiler expect this behaviour.  It's not about to change.
> 
> You have an option you can use, so use it.

Greetings,

I will accept this as proper closure for the bug [not the RESOLVED INVALID from before]. There are also other ways to work around the issue, such as by defining symbols in the linker script [how MSP430 does it]. At this point the compiler can't tell what is being dereferenced and make decisions based on that [which also explains why I did not run into this issue sooner]. I will however restate, you are likely [silently in some cases] breaking someone's code by enforcing this behaviour in bare metal though. Thank you.

Cheers,
Orlando.