Re: [PATCH 0/2] Problems with gdb.mi/mi-vla-fortran.exp

[Joel Brobecker <brobecker@adacore.com>]

Hello Andrew,

> Given the above mistake, what follows is just undefined behaviour
> kicking in; gdb reads the uninitialised memory, which happens to be
> non-zero, so it believes pvla2 is associated, it then tries to figure
> out what it's associated with, and reads yet more uninitialised memory.
> 
> In this end it figures that pvla2 is pointing at a huge 3G+ array,
> which in the gdb.mi/mi-vla-fortran.exp test we gdb to print.  This
> causes gdb to allocate the 3G+ of memory to hold the contents of the
> value.
> 
> My question is what to do next from the gdb side of things?
> 
> We could leave the test unchanged, arguing that gdb is asking sensible
> questions, and it's gfortran that's getting things wrong.
> 
> Or we could remove, or comment out, the offending test.
> 
> Leaving the test in place unchanged would be a bit of a shame, I know
> that 3G of memory isn't much on a lot of machines, but it's still
> making my experience running the testsuite pretty painful; I can't be
> the only one.
> 
> I think that this issue has highlighted a bigger issue that effects
> gdb, when dealing with languages that support dynamic types, like
> fortran.  An incorrect program can result in gdb having invalid type
> information, it would be nice if this invalid information did not
> cause gdb to do something so obviously wrong that it brings the
> maching running gdb down.
> 
> The following patches are my attempt to address this issue.  The first
> adds a mechansim to stop gdb allocating overly large arrays for value
> contents, the second enables this mechanism withn the test suite,
> while the third makes additional changes to the mi-vla-fortran.exp
> test to prevent additional errors that are a consequence of the above
> issue.
> 
> Thoughts and comments welcome.

Thanks for the detailed analysis!

I agree with you that we have to protect GDB against this kind of
bad data. As it happens, this used to happen all the time with Ada
programs, where dynamic data is common practice, especially back
in the days of stabs. For that, the Ada support code (in ada-lang.c)
introduced a setting called "varsize-limit", and we used it as
a limit for detecting probable issues in the debugging info.
I was sure that we had contributed that setting, but it turns out
that we contributed only the backbone (the code has access to that
setting), but forgot to contribute the command allowing the user to
change it! Arg... For the record, here is the hunk:

    @@ -14209,6 +14464,15 @@ With an argument, catch only exceptions
                         CATCH_TEMPORARY);

       varsize_limit = 65536;
    +  add_setshow_uinteger_cmd ("varsize-limit", class_support,
    +                           &varsize_limit, _("\
    +Set the maximum number of bytes allowed in a dynamic-sized object."), _("\
    +Show the maximum number of bytes allowed in a dynamic-sized object."), _("\
    +Attempts to access an object whose size is not a compile-time constant\n\
    +and exceeds this limit will cause an error."),
    +                           NULL, NULL, &setlist, &showlist);
    +  /* Add this alias to prevent ambiguity in 'set var ...' command. */
    +  add_alias_cmd ("var", "variable", class_vars, 1, &setlist);

       add_info ("exceptions", info_exceptions_command,
                _("\

So, all in all, I agree with the direction you'd like us to take.
I will followup on the patches you sent.

-- 
Joel