Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 329682 - [disassembly] Address bar does not work for symbols
Summary: [disassembly] Address bar does not work for symbols
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf (show other bugs)
Version: 8.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 8.0   Edit
Assignee: Patrick Chuong CLA
QA Contact: Pawel Piech CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-08 11:02 EST by Anton Leherbauer CLA
Modified: 2011-01-07 16:38 EST (History)
2 users (show)

See Also:
pchuong: iplog-

Attachments
fix (934 bytes, patch)
2010-11-08 11:11 EST, Patrick Chuong CLA 		no flags Details | Diff
evaluateSymbolAddress() API patch (20.51 KB, patch)
2010-11-10 17:28 EST, Patrick Chuong CLA 		pchuong: iplog-
Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Leherbauer CLA 2010-11-08 11:02:12 EST 
Entering 'main' in the address bar gives following exception (with DSF-GDB):

java.lang.NumberFormatException: For input string: "{int"
	at java.lang.NumberFormatException.forInputString(NumberFormatException.java:48)
	at java.lang.Integer.parseInt(Integer.java:449)
	at java.math.BigInteger.<init>(BigInteger.java:316)
	at java.math.BigInteger.<init>(BigInteger.java:451)
	at org.eclipse.cdt.debug.internal.ui.disassembly.dsf.DisassemblyUtils.decodeAddress(DisassemblyUtils.java:51)
	at org.eclipse.cdt.dsf.debug.internal.ui.disassembly.DisassemblyPart.eval(DisassemblyPart.java:2953)
	at org.eclipse.cdt.dsf.debug.internal.ui.disassembly.actions.JumpToAddressAction.run(JumpToAddressAction.java:45)

This seems to be caused by the changes for bug 326670.

Patrick, could you take a look?
Comment 1 Patrick Chuong CLA 2010-11-08 11:11:55 EST 
Created attachment 182630 [details]
fix
Comment 2 Patrick Chuong CLA 2010-11-08 11:51:51 EST 
Anton, can you please review the change?
Comment 3 Anton Leherbauer CLA 2010-11-09 02:18:00 EST 
(In reply to comment #2)
> Anton, can you please review the change?

The patch simply ignores the exception, but the issue is that navigating to 'main' and other symbols did work before.  JumpToAddressAction should use gotoSymbol().  That's what it is meant for.
Comment 4 Patrick Chuong CLA 2010-11-09 10:23:46 EST 
I need to remember the address in the combo box, do you have any suggestion how I can evaluate the expression ahead of time before calling gotoSymbol()?

can I evaluate for "symbol", if that returns an address, than use the address. Otherwise, evaluate "&symbol"?
Comment 5 Anton Leherbauer CLA 2010-11-10 08:54:33 EST 
(In reply to comment #4)
> I need to remember the address in the combo box, do you have any suggestion how
> I can evaluate the expression ahead of time before calling gotoSymbol()?

I guess this requires to extend the IDisassemblyBackend interface, e.g. gotoSymbol() could be split into evaluateSymbolAddress() and gotoAddress().
Comment 6 Patrick Chuong CLA 2010-11-10 09:36:31 EST 
I can modify the IDisassemblyBackend interface like you suggested and change both cdi and dsf implementations. 

There is another way around without changing the IDisassemblyBackend interface, by changing the eval(String expr) in DisassemblyPart to:

public BigInteger eval(String expr) {
    	String location = evaluateExpression(expr);
    	BigInteger result = getExpressionResult(location);
    	if (result == PC_UNKNOWN) {
    		location = evaluateExpression("&" + expr); //$NON-NLS-1$
    		result = getExpressionResult(location);
    	}
    	return result;
}

Can you let me know which way you think it works better?

Splitting up the gotoSymbol() is more flexibile and relies on the backend to evaluate the expression. However, this also means that IDisassemblyPartCallback interface needs to be extended to provide callback API for the backend to notify the DisassemblyPart the expression is evaluated. 

Modifying the eval API has less code change, but relies on the the client's knowledge of evalauting the expression, if failed, than evaluate the address of the expression.
Comment 7 Anton Leherbauer CLA 2010-11-10 09:56:48 EST 
The return value of IDisassemblyBackend.evaluateExpression() is just a string and its Javadoc says "Evaluate an expression for text hover", i.e. the returned string can be any text.  There is no guarantee that the returned string can be translated into an address.  Therefore enhancing IDisassemblyBackend is the only solution.
Comment 8 Patrick Chuong CLA 2010-11-10 17:28:22 EST 
Created attachment 182855 [details]
evaluateSymbolAddress() API patch

New evaluateSymbolAddress() API for IDisassemblyBackend, implemented for both dsf backend and cdi backend.
Comment 9 Anton Leherbauer CLA 2010-11-11 09:45:20 EST 
Looks good.
Please externalize the error message "Symbol does not evaluate to an address...".  I'd suggest to use the word "Location" or "Expression" instead of "Symbol".
The message key DisassemblyMessages.Disassembly_GotoAddressDialog_error_invalid_address is no longer used, we should remove it.

Feel free to commit if you already have the rights.
Thanks!
Comment 10 Patrick Chuong CLA 2010-11-11 12:14:45 EST 
Anton, I have committed the patch to CVS with the additional suggested changes in your last comment. Can you verify and close the bug? Thanks!

This is my first CVS commit, hope I didn't cause any trouble. :)
Comment 11 CDT Genie CLA 2010-11-11 12:23:04 EST 
*** cdt cvs genie on behalf of pchuong ***
Bug 329682 - [disassembly] Address bar does not work for symbols

[*] DisassemblyMessages.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyMessages.java?root=Tools_Project&r1=1.9&r2=1.10
[*] DisassemblyMessages.properties 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyMessages.properties?root=Tools_Project&r1=1.8&r2=1.9
[*] DisassemblyBackendDsf.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyBackendDsf.java?root=Tools_Project&r1=1.15&r2=1.16
[*] DisassemblyPart.java 1.36 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java?root=Tools_Project&r1=1.35&r2=1.36

[*] AddressBarContributionItem.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/actions/AddressBarContributionItem.java?root=Tools_Project&r1=1.2&r2=1.3
[*] JumpToAddressAction.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/actions/JumpToAddressAction.java?root=Tools_Project&r1=1.4&r2=1.5

[*] IDisassemblyBackend.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/disassembly/dsf/IDisassemblyBackend.java?root=Tools_Project&r1=1.2&r2=1.3
[*] DisassemblyBackendCdi.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/disassembly/dsf/DisassemblyBackendCdi.java?root=Tools_Project&r1=1.12&r2=1.13
Comment 12 Anton Leherbauer CLA 2010-11-12 08:11:26 EST 
(In reply to comment #10)
> Anton, I have committed the patch to CVS with the additional suggested changes
> in your last comment. Can you verify and close the bug? Thanks!
> 
> This is my first CVS commit, hope I didn't cause any trouble. :)

No complaints :)
Comment 13 Anton Leherbauer CLA 2010-11-12 12:50:41 EST 
I did realize that this would break the current version of TCFDisassemblyBackend, but I did not realize that would break the nightly CDT build.  I made the change backwards compatible by introducing an abstract implementation of IDisassemblyBackend.