In some places in the 1.1.x networking code we don't make sure that KEEP_POINTER_ALIVE macros are there.


This is the mail messages:

Many of the comments in the message from DEC is the result of
misunderstandings of how the 1.1/1.2 VM interacts with GC and C
compilers. There appears to be a bug in DEC's GC code:

> So, regarding the MCI bug, it could be that in fact the pointer to the
> object is in fact stashed on the stack, and we just don't pick it up
> during the gc scan.

The KEEP_POINTER_ALIVE macro is an adequate fix. It stashes
the pointer on the stack, but the GC scan must pick it up.
Something's wrong with their GC code if it's not picking up
all the pointers on stack.

In the long run, JavaSoft is systematically addressing the problem
in two ways:

1. Convert all native libraries to JNI. None of the JNI libraries
suffer from this problem. The new libraries are available in
1.2.

2. The next generation VM (HotSpot) will not reply on conservative
scanning.

Sheng

Jonathan Benoit - JavaSoft East wrote:
>
> hi all,
>
> OEM disagrees with KEEP_POINTER_ALIVE fix, they think this is a "pervasive and
> fundamental problem which needs to be addressed systematically by Javasoft".
>
> comments?
>
> Here are the details from OEM (DEC):
> -----------------------------------
>
> I) We think that, at best, KEEP_POINTER_ALIVE is a compiler
> dependent and/or machine dependent fix, and therefore a bad
> idea in any case and certainly not appropriate for machine
> independent code. Excerpt from a mail message to explain this
> further follows here:
>
> ...
>
> I think I see what he's trying to do here with KEEP_POINTER_ALIVE,
> but it's unclear to me whether it works in all cases.
>
> They're trying to insure that an "alive pointer" gets picked up
> within a register for a given call frame when the collector scans
> the C stack for pointers. By referencing the pointer the way
> they have, with the strange test for 0, they *hope* to keep the
> pointer laying around in the frame for a *previous* call, so that
> the collector won't move the object around.
>
> This seems bogus to me for a couple reasons:
>
> 1. The description for register v0 states clearly that it is
> not preserved around function calls. So in the case of
> a code sequence like the original case that we found:
>
> namearray = (HString **)unhand(namearrayhandle)->body;
> namearray[i] = makeJavaString(buffer, strlen(buffer));
>
> there is no guarentee that namearray[i] is anywhere in the
> context of the makeJavaString frame. namearray is probably
> hanging out on the stack some place similar to this:
>
> 0x120001258: 6b5b4d61 jsr ra, (t12), 0x1200047e0(zero)
> 0x12000125c: 27ba2000 ldah gp, 8192(ra)
> 0x120001260: 23bd6e34 lda gp, 28212(gp)
> 0x120001264: b41e0008 stq v0, 8(sp)
>
> But if the compiler has sufficient knowledge about the function(s),
> I would hypothesize that it *might* not always do this. It could
> also easily store it away in an offset-to-gp storage location
> that wouldn't get picked up during the scan.
>
>
> 2. Any assertion that something is guarenteed to be in an argument
> register is invalid on Alpha. "a0-a5: Used to pass the first
> six actual arguments. Not preserved across procedure calls."
>
> So, regarding the MCI bug, it could be that in fact the pointer to the
> object is in fact stashed on the stack, and we just don't pick it up
> during the gc scan. But it also might not be on the stack at all.
>
> II)
>
> Even if KEEP_POINTER_ALIVE were an adequate fix, we have so far found
> the following instances where 1) the same potential bug footprint
> occurs, i.e. an unhand followed by a memory operation that can provoke
> GC followed by a use of the unhanded pointer; 2) there is either no
> use of unhand or a suspicious one. This isn't a complete list: we're
> still looking.
>
> Definite bugs:
> file var func
> solaris/net/
> socket.c fdptr java_net_PlainSocketImpl_socketCreate
> socket.c sptr java_net_PlainSocketImpl_socketAccept
> socket.c sptr_fdptr java_net_PlainSocketImpl_socketAccept
> socket.c fdptr
> java_net_PlainDatagramSocketImpl_datagramSocketCreate
> share/java/lang/
> filesystem.c thisptr java_io_File_list0
> io.c fdptr java_io_RandomAccessFile_writeBytes
> src/solaris/sun/
> awt_Canvas.c cdata sun_awt_motif_MCanvasPeer_create
> awt_Choice.c odata sun_awt_motif_MChoicePeer_addItem
> awt_Choice.c cdata sun_awt_motif_MChoicePeer_setFont
> awt_Choice.c cdata sun_awt_motif_MChoicePeer_pReshape
> awt_Choice.c cdata sun_awt_motif_MChoicePeer_remove
> awt_FileDialog wdata sun_awt_motif_MFileDialogPeer_create
>
> Assuming X is a GC trigger point, also likely bugs:
> src/solaris/sun/
> awt_Button.c wdata sun_awt_motif_MButtonPeer_create
> awt_Button.c wdata sun_awt_motif_MButtonPeer_setLabel
> awt_Checkbox.c target sun_awt_motif_MCheckboxPeer_create
> awt_Checkbox.c wdata sun_awt_motif_MCheckboxPeer_create
> awt_Checkbox.c wdata sun_awt_motif_MCheckboxPeer_setLabel
> awt_Choice.c wdata sun_awt_motif_MChoicePeer_create
> awt_Component.c eventPtr sun_awt_motif_MComponentPeer_handleEvent
> awt_Component.c cdata sun_awt_motif_MComponentPeer_pInitialize
> awt_Component.c cdata sun_awt_motif_MComponentPeer_pShow
> awt_Component.c cdata sun_awt_motif_MComponentPeer_setFont
> awt_Component.c cdata sun_awt_motif_MComponentPeer_setCursor
> awt_Component.c bdata sun_awt_motif_MComponentPeer_setForeground
> awt_Component.c bdata sun_awt_motif_MComponentPeer_setBackground
> awt_Dialog.c targetPtr sun_awt_motif_MDialogPeer_create
> awt_Dialog.c parentData sun_awt_motif_MDialogPeer_create
> awt_Dialog.c wdata sun_awt_motif_MDialogPeer_pShow
> awt_Dialog.c wdata sun_awt_motif_MDialogPeer_pHide
> awt_Dialog.c wdata sun_awt_motif_MDialogPeer_pReshape
> awt_FileDialog targetPtr sun_awt_motif_MFileDialogPeer_create
> awt_FileDialog wdata sun_awt_motif_MFileDialogPeer_pShow
> awt_FileDialog wdata sun_awt_motif_MFileDialogPeer_setFileEntry
> awt_FileDialog tdata sun_awt_motif_MFileDialogPeer_setFont
>
> Relies on dodgy KEEP_POINTER_ALIVE macro:
> share/java/lang/
> io.c dataptr java_io_FileOutputStream_writeBytes
> io.c dataptr java_io_RandomAccessFile_writeBytes
> io.c dataptr java_io_FileOutputStream_writeBytes
>
> What needs to be done to escalate this?


hi all,

oem disagrees with keep_pointer_alive fix, they think this is a "pervasive and
fundamental problem which needs to be addressed systematically by javasoft".

comments?


here are the details from oem (dec):
-----------------------------------

i) we think that, at best, keep_pointer_alive is a compiler
dependent and/or machine dependent fix, and therefore a bad
idea in any case and certainly not appropriate for machine
independent code. excerpt from a mail message to explain this
further follows here:

...

i think i see what he's trying to do here with keep_pointer_alive,
but it's unclear to me whether it works in all cases.

they're trying to insure that an "alive pointer" gets picked up
within a register for a given call frame when the collector scans
the c stack for pointers. by referencing the pointer the way
they have, with the strange test for 0, they *hope* to keep the
pointer laying around in the frame for a *previous* call, so that
the collector won't move the object around.

this seems bogus to me for a couple reasons:

  1. the description for register v0 states clearly that it is
      not preserved around function calls. so in the case of
      a code sequence like the original case that we found:

        namearray = (hstring **)unhand(namearrayhandle)->body;
        namearray[i] = makejavastring(buffer, strlen(buffer));

      there is no guarentee that namearray[i] is anywhere in the
      context of the makejavastring frame. namearray is probably
      hanging out on the stack some place similar to this:

  0x120001258: 6b5b4d61 jsr ra, (t12), 0x1200047e0(zero)
  0x12000125c: 27ba2000 ldah gp, 8192(ra)
  0x120001260: 23bd6e34 lda gp, 28212(gp)
  0x120001264: b41e0008 stq v0, 8(sp)

      but if the compiler has sufficient knowledge about the function(s),
      i would hypothesize that it *might* not always do this. it could
      also easily store it away in an offset-to-gp storage location
      that wouldn't get picked up during the scan.

     
  2. any assertion that something is guarenteed to be in an argument
      register is invalid on alpha. "a0-a5: used to pass the first
      six actual arguments. not preserved across procedure calls."

so, regarding the mci bug, it could be that in fact the pointer to the
object is in fact stashed on the stack, and we just don't pick it up
during the gc scan. but it also might not be on the stack at all.

ii)

even if keep_pointer_alive were an adequate fix, we have so far found
the following instances where 1) the same potential bug footprint
occurs, i.e. an unhand followed by a memory operation that can provoke
gc followed by a use of the unhanded pointer; 2) there is either no
use of unhand or a suspicious one. this isn't a complete list: we're
still looking.


definite bugs:
file var func
solaris/net/
socket.c fdptr java_net_plainsocketimpl_socketcreate
socket.c sptr java_net_plainsocketimpl_socketaccept
socket.c sptr_fdptr java_net_plainsocketimpl_socketaccept
socket.c fdptr
java_net_plaindatagramsocketimpl_datagramsocketcreate
share/java/lang/
filesystem.c thisptr java_io_file_list0
io.c fdptr java_io_randomaccessfile_writebytes
src/solaris/sun/
awt_canvas.c cdata sun_awt_motif_mcanvaspeer_create
awt_choice.c odata sun_awt_motif_mchoicepeer_additem
awt_choice.c cdata sun_awt_motif_mchoicepeer_setfont
awt_choice.c cdata sun_awt_motif_mchoicepeer_preshape
awt_choice.c cdata sun_awt_motif_mchoicepeer_remove
awt_filedialog wdata sun_awt_motif_mfiledialogpeer_create

assuming x is a gc trigger point, also likely bugs:
src/solaris/sun/
awt_button.c wdata sun_awt_motif_mbuttonpeer_create
awt_button.c wdata sun_awt_motif_mbuttonpeer_setlabel
awt_checkbox.c target sun_awt_motif_mcheckboxpeer_create
awt_checkbox.c wdata sun_awt_motif_mcheckboxpeer_create
awt_checkbox.c wdata sun_awt_motif_mcheckboxpeer_setlabel
awt_choice.c wdata sun_awt_motif_mchoicepeer_create
awt_component.c eventptr sun_awt_motif_mcomponentpeer_handleevent
awt_component.c cdata sun_awt_motif_mcomponentpeer_pinitialize
awt_component.c cdata sun_awt_motif_mcomponentpeer_pshow
awt_component.c cdata sun_awt_motif_mcomponentpeer_setfont
awt_component.c cdata sun_awt_motif_mcomponentpeer_setcursor
awt_component.c bdata sun_awt_motif_mcomponentpeer_setforeground
awt_component.c bdata sun_awt_motif_mcomponentpeer_setbackground
awt_dialog.c targetptr sun_awt_motif_mdialogpeer_create
awt_dialog.c parentdata sun_awt_motif_mdialogpeer_create
awt_dialog.c wdata sun_awt_motif_mdialogpeer_pshow
awt_dialog.c wdata sun_awt_motif_mdialogpeer_phide
awt_dialog.c wdata sun_awt_motif_mdialogpeer_preshape
awt_filedialog targetptr sun_awt_motif_mfiledialogpeer_create
awt_filedialog wdata sun_awt_motif_mfiledialogpeer_pshow
awt_filedialog wdata sun_awt_motif_mfiledialogpeer_setfileentry
awt_filedialog tdata sun_awt_motif_mfiledialogpeer_setfont

relies on dodgy keep_pointer_alive macro:
share/java/lang/
io.c dataptr java_io_fileoutputstream_writebytes
io.c dataptr java_io_randomaccessfile_writebytes
io.c dataptr java_io_fileoutputstream_writebytes

what needs to be done to escalate this?




benjamin.renaud@Eng 1997-10-06