Untangling Goto Statements (e.g. for replacement in Java)

Question

Edit: I apologize for not explicating this. I am well aware that goto is not currently a functional part of the Java language. This question is not about trying to use goto in Java, it's about trying to not use it.

Edit: And, while I do appreciate help in the specific case given, this question is more looking towards whether there are deterministic methods applicable to the process of refactoring such code.

Hello all,

This is a question similar to this one but seeking to be more general.

In short, I am decompiling and modifying a class file, and I want to recompile it again for use, but I get ~25 errors, most of them relating to the use of goto and related named blocks of code.

After looking around for quite some time, it seems my choices are to either pull a better decompiler than the ones I used out of the fabric of the internet, or untangle it myself and see if what I can make will compile properly.

My question is, are there any general guidelines that would be good to follow when attempting to convert goto statements into more sane/commonly accepted control flow keywords? So far it seems like recommendations have been on a case-by-case basis; are there ideas that can be applied across such exercises?

In case it helps, I'll post the relevant code here:

 private void unpackNatives(CompleteVersion version, File targetDir)
        throws IOException
    {
        OperatingSystem os;
        Iterator iterator;
        os = OperatingSystem.getCurrentPlatform();
        Collection libraries = version.getRelevantLibraries();
        iterator = libraries.iterator();
          goto _L1
_L7:
        ZipFile zip;
        ExtractRules extractRules;
        Library library = (Library)iterator.next();
        Map nativesPerOs = library.getNatives();
        if(nativesPerOs == null || nativesPerOs.get(os) == null)
            continue; /* Loop/switch isn't completed */
        File file = new File(Launcher.getInstance().getBaseDirectory(), (new StringBuilder("libraries/")).append(library.getArtifactPath((String)nativesPerOs.get(os))).toString());
        zip = new ZipFile(file);
        extractRules = library.getExtractRules();
        Enumeration entries = zip.entries();
          goto _L2
_L5:
        BufferedInputStream inputStream;
        byte buffer[];
        FileOutputStream outputStream;
        BufferedOutputStream bufferedOutputStream;
        ZipEntry entry = (ZipEntry)entries.nextElement();
        if(extractRules != null && !extractRules.shouldExtract(entry.getName()))
            continue; /* Loop/switch isn't completed */
        File targetFile = new File(targetDir, entry.getName());
        if(targetFile.getParentFile() != null)
            targetFile.getParentFile().mkdirs();
        if(entry.isDirectory())
            continue; /* Loop/switch isn't completed */
        inputStream = new BufferedInputStream(zip.getInputStream(entry));
        buffer = new byte[2048];
        outputStream = new FileOutputStream(targetFile);
        bufferedOutputStream = new BufferedOutputStream(outputStream);
        int length;
        while((length = inputStream.read(buffer, 0, buffer.length)) != -1) 
            bufferedOutputStream.write(buffer, 0, length);
          goto _L3
        Exception exception;
        exception;
        Downloadable.closeSilently(bufferedOutputStream);
        Downloadable.closeSilently(outputStream);
        Downloadable.closeSilently(inputStream);
        throw exception;
_L3:
        Downloadable.closeSilently(bufferedOutputStream);
        Downloadable.closeSilently(outputStream);
        Downloadable.closeSilently(inputStream);
_L2:
        if(entries.hasMoreElements()) goto _L5; else goto _L4
_L4:
        break MISSING_BLOCK_LABEL_339;
        Exception exception1;
        exception1;
        zip.close();
        throw exception1;
        zip.close();
_L1:
        if(iterator.hasNext()) goto _L7; else goto _L6
_L6:
    }

This is my first question on SO, so help with format/tags is most welcome.

goto has no functionality in Java - it's a reserved keyword, but doesn't do anything and so you can't use it as it appears in the source. — Michael Berry, May 20 '14 at 00:46
You simply need to understand how an if/else, for, while, do/while, or switch statement would translate into assembly language, figure out which sort of statement you have, and translate it back. It's only work, no magic. — Hot Licks, May 20 '14 at 00:48
Eg, the very last executable statement above appears to be the while statement of a do/while spanning most of the method. But if you look closer there is the jump from near the top to L1, so it's really a simple while statement. — Hot Licks, May 20 '14 at 00:52
If it were me, I'd print it out, then draw some lines showing where the `goto`'s go (don't bother with things like the last `goto _L6` which doesn't go anywhere, really). This would help visualize which sequences look like `if/else`, which ones look like loops, etc. Hopefully it hasn't been mashed up by an optimizer which could rearrange everything. — ajb, May 20 '14 at 00:52
Thanks for the responses, all. @HotLicks I think that's the sort of answer I was looking for. Does that imply that such control flow patterns will always translate into and out of assembly the same way? And, before I go searching off on my own, do you know of any particular good sources for learning about this process? Thanks again! — Korigan Stone, May 22 '14 at 22:22
@ajb Thanks for the recommendation, that's a great idea! Your statement about an optimizer concerns me though, because I can't be sure one isn't at play. Could you clarify a bit more? What sort of changes might an optimizer make? Thanks again. — Korigan Stone, May 22 '14 at 22:22
One example: If you have a nested loop, the inner loop might contain a "goto" that jumps to the end of inner loop, but the outer loop might immediately execute a "goto" that jumps to the beginning of the outer loop. You could still look at the goto's and figure out the structure. But an optimizer would realize that having two goto's in a row is redundant and change the first one, and now that makes it harder to see the control structure from the goto's. That's a simple case; a good optimizer can do lots of stuff to squeeze out unnecessary instructions. — ajb, May 22 '14 at 22:29
Compilers will generally generate similar object code patterns for similar high-level constructs. So once you become familiar with the patterns of a given compiler you can sometimes "see" the source code at a glance. Optimizers can muck this up big time, but very little optimization occurs in *javac* (mostly because the JVM specs don't allow it) -- Java optimization is left to the JITC and so the bytecodes are left "unmolested". — Hot Licks, May 22 '14 at 22:37

Sky · Answer 1 · 2014-05-20T03:00:00.590

private void unpackNatives(CompleteVersion version, File targetDir)
        throws IOException {
        OperatingSystem os;
        Iterator iterator;
        os = OperatingSystem.getCurrentPlatform();
        Collection libraries = version.getRelevantLibraries();
        iterator = libraries.iterator();
        if(iterator.hasNext()){ // L1
            ZipFile zip; // L7
            ExtractRules extractRules;
            Library library = (Library)iterator.next();
            Map nativesPerOs = library.getNatives();
            if(nativesPerOs == null || nativesPerOs.get(os) == null)
                continue; /* Loop/switch isn't completed */
            File file = new File(Launcher.getInstance().getBaseDirectory(), (new StringBuilder("libraries/")).append(library.getArtifactPath((String)nativesPerOs.get(os))).toString());
            zip = new ZipFile(file);
            extractRules = library.getExtractRules();
            Enumeration entries = zip.entries(); 
            if(entries.hasMoreElements()){ // L2
                     BufferedInputStream inputStream; // L5
                     byte buffer[];
                     FileOutputStream outputStream;
                     BufferedOutputStream bufferedOutputStream;
                     ZipEntry entry = (ZipEntry)entries.nextElement();
                     if(extractRules != null && !extractRules.shouldExtract(entry.getName()))
                         continue; /* Loop/switch isn't completed */
                     File targetFile = new File(targetDir, entry.getName());
                     if(targetFile.getParentFile() != null)
                         targetFile.getParentFile().mkdirs();
                     if(entry.isDirectory())
                         continue; /* Loop/switch isn't completed */
                     inputStream = new BufferedInputStream(zip.getInputStream(entry));
                     buffer = new byte[2048];
                     outputStream = new FileOutputStream(targetFile);
                     bufferedOutputStream = new BufferedOutputStream(outputStream);
                     int length;
                     while((length = inputStream.read(buffer, 0, buffer.length)) != -1) 
                        bufferedOutputStream.write(buffer, 0, length);
                     Downloadable.closeSilently(bufferedOutputStream); // L3
                     Downloadable.closeSilently(outputStream);
                     Downloadable.closeSilently(inputStream);
                     Exception exception;
                     exception;
                     Downloadable.closeSilently(bufferedOutputStream);
                     Downloadable.closeSilently(outputStream);
                     Downloadable.closeSilently(inputStream);
                     throw exception;
                } else { // L4
                    break MISSING_BLOCK_LABEL_339; 
                    Exception exception1;
                    exception1;
                    zip.close();
                    throw exception1;
                    zip.close();
                }
            }
        } // L6

It seems to me that it should be written something like this after untangling. I replaced all the goto line reference as Java doesn't support goto. Goto is arguable bad coding as most coders agree that a good way of coding doesn't requires goto. Basically 'goto' is just jumping to the reference code. In low level memory you probably seeing something like

jmp <address path to whatever reference>

Therefore to untangle it, just reference to whichever it get pointed to and reading a bit of if/else code.

@LawrenceDol I noticed that but base on the comments on the code, seems to me it's an incomplete code. And the OP is not questioning on the loop but on the goto. — Sky, May 20 '14 at 02:57
@LawrenceDol and I would like to question how would I go about to implement the loop base on the information the OP has given? — Sky, May 20 '14 at 03:05
The line, `if(iterator.hasNext()){ // L1` should be `while(iterator.hasNext()){ // L1` — Lawrence Dol, May 20 '14 at 04:20
Thanks to both of you for the help. Sky, that looks better, coupled with Lawrence's suggestion, applied to both L1 and L2. The question was also asking after, what process did you do to decide that? I came to much the same conclusion, but was wondering if there are deterministic methods applicable to such a process. — Korigan Stone, May 22 '14 at 22:39
@KoriganStone it's like piecing up jigsaw puzzle by referencing to the correct pointer. @LawrenceDol mention it's a "while" in this case is due to this piece of code -> `Library library = (Library)iterator.next();`, although your original L1 code should mention "while" instead of "if", not sure why it happens that though. Probably its original coding is wrong or the decompiler is not accurate. — Sky, May 23 '14 at 00:49

Untangling Goto Statements (e.g. for replacement in Java)

1 Answers1