On 03/29/2015 01:30 PM, Sandra Loosemore wrote:
As the outcome of this discussion last month about unexpected behavior
of -fisolate-erroneous-paths-dereference
https://gcc.gnu.org/ml/gcc/2015-02/msg00163.html
Altera has asked us to turn off -fdelete-null-pointer-checks on
nios2-elf targets. Presently, the AVR and CR16 back ends completely
disable this option, but it seems to me that a better solution is to
simply default it to off. That's what the attached patch does for
nios2, with appropriate adjustments to the testsuite.
The rationale for defaulting to -fno-delete-null-pointer-checks on a
bare-metal target is that it is the "safe" setting on targets where 0 is
a valid memory address, and where the hardware may even require
placement of e.g. a reset vector at that address. Also, library code
should be built to be "safe", especially things like memcpy that might
be used by implicit compiler-generated calls.
The rationale for allowing the default to be overridden on the command
line, instead of disabling -fdelete-null-pointer-checks entirely, is
that it's much more common for programs *not* to manipulate code or data
at address 0, and we can generate smaller/faster code by enabling the
option.
So....
(1) Is the change to the default for this flag in common.opt OK? I
verified that all existing uses just check for zero/non-zero-ness.
(2) If the AVR and CR16 maintainers agree that defaulting the option to
off rather than being completely disabling it is a better solution for
their targets, too, I think being consistent across the three targets
would simplify ongoing support -- e.g., the testsuite bits could be
greatly simplified by getting rid of
check_effective_target_keeps_null_pointer_checks and checks for
keeps_null_pointer_checks entirely. WDYT?
I haven't done a full bootstrap with this patch yet.... I'd just like
to get it out there for discussion to see if there is consensus that
this is a reasonable approach, first. This is stage 1 material in any
case.
It looks very sane to me. This is probably how the AVR and CR16 should
have been handled to begin with IMHO.
FWIW, I generally discourage ports overriding default options, but this
is a case where I believe it makes some sense.
Please move forward with an official submission.
jeff