Bug 432597 - GDB Memory services should report address size in octets (not bytes)
Summary: GDB Memory services should report address size in octets (not bytes)
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf-gdb (show other bugs)
Version: 8.3.0   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 8.4.0   Edit
Assignee: Teodor Madan CLA
QA Contact: Marc Khouzam CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-11 04:45 EDT by Teodor Madan CLA
Modified: 2014-04-25 11:29 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Teodor Madan CLA 2014-04-11 04:45:17 EDT
Currently user cannot scroll to the last addressable byte when addressable sizes > 1 octet, due to IMemoryBlockExtension#getAddressSize() returning bytes instead of octets. 

------------------------------
Debug Platform memory API specifies "bytes" with the implicit reference to MemoryByte. The later is clearly a mapping to java byte (i.e. for CDT would be "octets").

All the memory renderings implementation to make the same assumption, i.e. IMemoryBlockExtension#getAddressableSize() and IMemoryBlockExtension# getAddressSize() return "octets".

The two equivalent method from GDB API, IGDBMemory.getAddressSize() and IGDBMemory2#getAddressableSize, are inconsistent with this regard. Addressable size should be reported as "octets", while address size is reported as "bytes", the later being subject to interpretations.

Proposal is to change IGDBMemory.getAddressSize() to report "octets".
Comment 1 Teodor Madan CLA 2014-04-11 08:55:14 EDT
Patch for review in https://git.eclipse.org/r/#/c/24849/
Comment 2 Alvaro Sanchez-Leon CLA 2014-04-11 09:16:11 EDT
(In reply to Teodor Madan from comment #1)
> Patch for review in https://git.eclipse.org/r/#/c/24849/

Thanks for posting this update, 
The change looks good in general although it requires some testing in a target system with minimum addressable size larger than one octet.
Comment 3 Teodor Madan CLA 2014-04-11 09:29:37 EDT
(In reply to Alvaro Sanchez-Leon from comment #2)
> Thanks for posting this update, 
> The change looks good in general although it requires some testing in a
> target system with minimum addressable size larger than one octet.

I could not find a gdb port for such arch. By any chance are you aware of any such port that I could use for smoke testing.

Thank you,
Teo
Comment 4 Alvaro Sanchez-Leon CLA 2014-04-11 09:33:43 EDT
> I could not find a gdb port for such arch. By any chance are you aware of
> any such port that I could use for smoke testing.
> 

Not really, although I have access to a proprietary system that I could use to test it, I would not be able to go through it now but after a couple of weeks.
Comment 5 Marc Khouzam CLA 2014-04-11 18:52:22 EDT
(In reply to Teodor Madan from comment #3)
> (In reply to Alvaro Sanchez-Leon from comment #2)
> > Thanks for posting this update, 
> > The change looks good in general although it requires some testing in a
> > target system with minimum addressable size larger than one octet.
> 
> I could not find a gdb port for such arch. By any chance are you aware of
> any such port that I could use for smoke testing.

Teodor, I assume that if you noticed the problem which happens on targets with addressable sizes > 1, you may have access to a proprietary target of that kind.  Is that not the case?

Like Alvaro, I think this change makes sense.  But we have to check that all the calls to getAddressSize() expect octets.  We really should aim to get that fixed for Luna though.
Comment 6 Teodor Madan CLA 2014-04-14 04:30:12 EDT
(In reply to Marc Khouzam from comment #5)
> Teodor, I assume that if you noticed the problem which happens on targets
> with addressable sizes > 1, you may have access to a proprietary target of
> that kind.  Is that not the case?
I do *not* have access to a gdb port of such proprietary target. The problem in GDB backend was noticed when reviewing Bug 432254 .

> Like Alvaro, I think this change makes sense.  But we have to check that all
> the calls to getAddressSize() expect octets.  We really should aim to get
> that fixed for Luna though.
getAddressSize() is used only through IMemoryBlockExtension#getAddressSize(). Re-reviewing the usage of API confirms experience I had in adding multi-byte addressable support in CDI based debugger backend. 

After addressing review comments, I would push to head fix for inconsistency in IGDBMemory API. API clarification should be released for 8.4. 

Alvaro, are you willing to validate the issue later on when you have the time/resources?
Comment 7 Alvaro Sanchez-Leon CLA 2014-04-14 08:44:44 EDT
(In reply to Teodor Madan from comment #6)

> Alvaro, are you willing to validate the issue later on when you have the
> time/resources?

Yes, I will test it within a couple of weeks.
Comment 8 Teodor Madan CLA 2014-04-17 09:57:27 EDT
I've pushed patch to master and will leave open issue for now, until being able to validate.

http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=839905a802e6d6562b08cfb9ec5d0513764c1ccd
Comment 9 Alvaro Sanchez-Leon CLA 2014-04-25 10:10:10 EDT
I have now validated the change, 
It works as expected using octets instead of addressable units (bytes)
Comment 10 Teodor Madan CLA 2014-04-25 11:29:04 EDT
Thank you.