[PATCH, Fortran] Fix PR36112: Bounds-checking on character-array-constructors

[FX]

Hi Daniel,

> For array-constructors with all-constant string lengths, the compiler
> pads the shorter strings to the maximum of all lengths', thus  
> preventing
> a runtime-error on different string lengths.  In this case, my  
> patch now
> emits a compiler-error for -fbounds-check.

-fbounds-check says it "enables generation of run-time checks". It  
should not have any visible influence at compile-time. If the code is  
determined to be illegal at compile-time, we should emit an error  
unconditionaly. If the code can't be determined at compile-time, we  
will do nothing if -fbounds-check is not given, and we will add run- 
time checks if -fbounds-check is used (thus slowly down runtime  
performance considerably, as you can imagine).

> My suggested reason for this is that trans- 
> array.c:get_array_ctor_strlen
> used to return the length of the last element; changing this behaviour
> to returning the length of the first element fixes the bounds-checking
> problem and does not introduce any regressions.

That seems like a good plan, then :)

> 1) Is it ok to give the new error message for all-constant string
> lengths and -fbounds-check?  I could maybe replace it by a warning or
> try to find some other means, like not adjusting the string lengths  
> and
> relying on the runtime-check; but I believe this is the best solution.

See above; all we can do at compile-time should be done there, as  
runtime errors are always more painful to the user (and thus, almost  
all of them are hidden behind a specific flag, -fbounds-check).  
Issuing an error at compile is perfectly fine and desirable, because  
the code is invalid: we ought to diagnose as much invalid code as we  
can.

> 2) In order to make this error reporting work I had to remove a  
> call to
> gfc_resolve_character_array_constructor from
> gfc_resolve_array_constructor (marked by XXX at the moment), as
> gfc_resolve_character_array_constructor would otherwise be called  
> twice
> resulting in a duplicate error message; as far as I can tell, this  
> call
> was superfluous as gfc_resolve_character_array_constructor is  
> called in
> the (only) caller of gfc_resolve_array_constructor some lines later
> anyway.

That seems perfectly fine.


I have one more question: you add special cases for  
get_array_ctor_var_strlen call() to allow it to be called with len ==  
NULL, but why? When (and where) is it called with a NULL argument?

FX


-- 
François-Xavier Coudert
http://www.homepages.ucl.ac.uk/~uccafco/