This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch] Java: Add heap dump and analyze support.


>>>>> "David" == David Daney <ddaney@avtrex.com> writes:

Sorry for the delay on this.

David> This patch adds the ability to dump the contents of the runtime heap
David> to a file from a java program.

Nice.

I read through this.  I don't see many issues -- a few formatting
things (on irc you said these are fixed).

I'm a little concerned about the ad hoc approach to demangling, but
not enough to want to change it.

I'm also worried about the GC private bits copied into the native
code.  My main concern is that if the GC changes, we won't notice.

My final high-level worry is about security.  Right now it is possible
for any program to call the code that dumps the memory map.  I suppose
we can just sweep this under the blanket we're putting 'gnu.classpath'
stuff under.

Please comment on these.


I have a few more minor comments.

David> +@item -p @var{tool-prefix}
David> +Prefix added to the names of the @command{nm} and @command{readelf} commands.

I'd prefer to compile in the target, but I suppose this tool isn't
built in non-native builds, so that wouldn't help.  Darn.

David> +  // search all symbol tables
David> +  // return -1 if none found
David> +  long getAddress(SymbolLookup lookup, String symbol) throws IOException
David> +  {
David> +    //    for (Iterator it=map.entrySet().iterator(); it.hasNext(); )
David> +    //      {

Why is this commented out?

David> +        if (s.charAt(0) == '#')
David> +          continue;
David> +        if (s == null)
David> +          break;

These tests are in the wrong order.

David> +  static void usage()
David> +  {

Someday I suppose we should convert the tools in gcj to use the
classpath getopt code.

David> +              if (name.compareTo("gnu.gcj.runtime.MethodRef") == 0)

What is this?

David> +  public static String KindToName(int kind)

Non-java name... method names should start with a lower case letter.

David> +namespace gnu
David> +{
David> +  namespace gcj
David> +  {
David> +    namespace util
David> +    {
David> +      class GC_enumerator

This appears to be local to a single .cc file.  Do we need to put it
into a namespace like this?  I've always thought of these namespaces
as reserved for 'extern "Java"' code.

Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]