Re: [PATCH v4] Make chained function calls in expressions work

[Siva Chandra <sivachandra at google dot com>]

On Tue, Nov 4, 2014 at 5:38 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Yes, having an additional flag in struct expression would fix the safety
> issue.  Moving initialization to evalute_subexp if *pos == 0 would then
> no longer be safety issue, but simply enabling use of temporaries in more
> cases.

Since I have my code already setup in this fashion, I would prefer to
go this route unless you see an advantage of going with the solution
you suggest below.

> The other option I've been thinking of would be to move the whole handling
> of temporaries to infcall.c, by providing two functions there, e.g. called:
>
>   infcall_enable_on_stack_temporaries ()
>   infcall_cleanup_on_stack_temporaries ()
>
> These would use a new field in the inferior_thread () thread struct to
> store the vector of currently in use on-stack temporaries.  Any call to
> call_function_by_hand after infcall_enable_on_stack_temporaries (and
> before infcall_cleanup_on_stack_temporaries) on the same thread would
> be allowed to use temporaries.  [ There could be sanity checks like
> having call_function_by_hand verify that the current SP still equals
> the thread SP at the time infcall_enable_on_stack_temporaries was
> called.  Also, infrun could assert that a thread that has temporaries
> in use must only ever run during is_infcall, to catch missing cleanup
> calls.  ]
>ReadStringAndDumpToStream<StringElementType::ASCII> (ReadStringAndDumpToStreamOptions options)
> The only thing evaluate_expression would then need to do is to call
> infcall_enable_on_stack_temporaries and install a cleanup calling
> infcall_cleanup_on_stack_temporaries.

[...]

> So I'm thinking of the following sequence of events:
>
> - We call the first call_function_by_hand with a current sp value SP
>   (no pre-existing temporaries at this point).
> - We have to reserve the red zone, setting sp e.g. to SP - 256.
> - We now allocate a temporary, taking up e.g. SP - 256 .. SP - 512.
> - First call_function_by_hand returns.
> - We get a second call_function_by_hand call.
> - Now we have a temporary on the stack from  SP - 256 .. SP - 512,
>   so skip_current_expression_stack skips down to SP - 512.
> - *Now* code allocates the red zone again, setting SP to SP - 768.
>   Even though this is unnecessary; the red zone is actually already
>   there in the range SP ... SP - 256.
>
> Bascially, adding a red zone at any point other the original SP isget_xmethod_arg_types
> useless, since the only point of the red zone is to protect data that
> the original inferior code might have written to its stack immediately
> below the original SP.

Oops! Sorry, I misread my own code. You are right and I will fix it.

> You mean "Any two or more lines in code should be wrapped in braces, even
> if they are comments, as they look like separate statements"?  I don't think
> this is intended to apply to cases like the above where a single statement
> just had to split into multiple lines since it doesn't fit into one.
> This case will never "look like separate statements".  In any case, all
> the existing precedent in GDB does not have extra braces in that case.

Sorry for picking this. I point out because I was asked to put braces
for cases like this in the past. Example:
extension.c:get_xmethod_arg_types.