Cygwin GDB crashes from cvs - solib

Cygwin GDB crashes from cvs - solib

[Daniel Jacobowitz]

Currently win32-nat.c sets in_dynsym_resolve_code and open_symbol_file
to NULL.  The latter doesn't normally get called, but when it does,
it is called unconditionally; the former is called by "step".  Should
win32-nat provide dummy functions or should the call sites check?  Anyone
have an opinion?

-- 
Daniel Jacobowitz
CodeSourcery

RE: Cygwin GDB crashes from cvs - solib

[Dave Korn]

On 10 April 2006 18:21, Daniel Jacobowitz wrote:

> Currently win32-nat.c sets in_dynsym_resolve_code and open_symbol_file
> to NULL.  The latter doesn't normally get called, but when it does,
> it is called unconditionally; the former is called by "step".  Should
> win32-nat provide dummy functions or should the call sites check?  Anyone
> have an opinion?

  Well, IIUIC, we have various target vectors of different kinds throughout
gcc binutils and gdb, and in every case that I can bring to mind off the top
of my head, there's no requirement that every single entry has to be
initialised, and a NULL entry indicates 'target does not have capability'.

  So from that point of view, ISTM that no call site should blindly jump
through a target vector pointer without first checking that it is non-NULL,
unless it already 'knows' by some other means that the current target /has/ to
have the capability in question (in which case if the pointer is NULL it's a
programming bug because the information in the target vector must not be
semantically self-consistent).

  IASTM that if we start requiring all function-pointers in target vectors to
be filled out with a pointer to a dummy function if there is no real function
for the target, we lose the ability to test if the target has the given
capability, and it would, in time, lead to people being tempted to write
really ugly code such as:-

  if (current_target_so_ops->open_symbol_file == &dummy_open_symbol_file_stub)
    ... assume we don't have the capability ...
  else 
    ... assume we do ...

which wouldn't be a good thing IMO.

  However that's a generic POV on the general issue.  Cgf will probably have a
relevant opinion about this particular problem since he's been paying some
attention to the win32 native solib stuff lately and I would defer to his
judgement on this one.

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: Cygwin GDB crashes from cvs - solib

[Christopher Faylor]

[reply-to set]
On Mon, Apr 10, 2006 at 06:47:34PM +0100, Dave Korn wrote:
>On 10 April 2006 18:21, Daniel Jacobowitz wrote:
>>Currently win32-nat.c sets in_dynsym_resolve_code and open_symbol_file
>>to NULL.  The latter doesn't normally get called, but when it does, it
>>is called unconditionally; the former is called by "step".  Should
>>win32-nat provide dummy functions or should the call sites check?
>>Anyone have an opinion?
>
>Well, IIUIC, we have various target vectors of different kinds
>throughout gcc binutils and gdb, and in every case that I can bring to
>mind off the top of my head, there's no requirement that every single
>entry has to be initialised, and a NULL entry indicates 'target does
>not have capability'.
>
>So from that point of view, ISTM that no call site should blindly jump
>through a target vector pointer without first checking that it is
>non-NULL, unless it already 'knows' by some other means that the
>current target /has/ to have the capability in question (in which case
>if the pointer is NULL it's a programming bug because the information
>in the target vector must not be semantically self-consistent).
>
>IASTM that if we start requiring all function-pointers in target
>vectors to be filled out with a pointer to a dummy function if there is
>no real function for the target, we lose the ability to test if the
>target has the given capability, and it would, in time, lead to people
>being tempted to write really ugly code such as:-
>
>if (current_target_so_ops->open_symbol_file ==
>&dummy_open_symbol_file_stub) ...  assume we don't have the capability
>...  else ...  assume we do ...
>
>which wouldn't be a good thing IMO.
>
>However that's a generic POV on the general issue.  Cgf will probably
>have a relevant opinion about this particular problem since he's been
>paying some attention to the win32 native solib stuff lately and I
>would defer to his judgement on this one.

I don't think you really need my opinion on this.  The above reasoning
seems to me to be 100% correct.

Whether win32-nat.c should add these two functions (or any others that
may be missing) is really another problem entirely.

cgf

Re: Cygwin GDB crashes from cvs - solib

['Daniel Jacobowitz']

On Mon, Apr 10, 2006 at 06:47:34PM +0100, Dave Korn wrote:
>   However that's a generic POV on the general issue.  Cgf will probably have a
> relevant opinion about this particular problem since he's been paying some
> attention to the win32 native solib stuff lately and I would defer to his
> judgement on this one.

Actually, if anything, I'm hoping for a response from Kevin :-)

What you say is true for gdbarch.  It is not quite true for target
vectors, and I don't know of any other prior art for solibs.

-- 
Daniel Jacobowitz
CodeSourcery

RE: Cygwin GDB crashes from cvs - solib

[Dave Korn]

On 10 April 2006 19:02, 'Daniel Jacobowitz' wrote:

> What you say is true for gdbarch.  It is not quite true for target
> vectors, and I don't know of any other prior art for solibs.

  Sorry, slight contextual laxness with the term "target vector" there, the
gdbarch is in fact what I was specifically thinking of when I wrote that,
although I was also thinking of bfd and gcc, which also have "target vectors",
and which generally require the pointers to be tested before use.


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: Cygwin GDB crashes from cvs - solib

['Daniel Jacobowitz']

On Mon, Apr 10, 2006 at 02:00:10PM -0400, Christopher Faylor wrote:
> Whether win32-nat.c should add these two functions (or any others that
> may be missing) is really another problem entirely.

You seem to have done this?

Thanks for fixing the bug, but please remember to post patches to
gdb-patches.

-- 
Daniel Jacobowitz
CodeSourcery