#1250 Enhance C::B GDB memory watch class for future changes

[Andrew Cottrell]

These changes are to allow plugins to be able to request GDB memory access via a symbol, which does not exist at the moment. The existing memory dump dialog uses functions that are not exposed in the SDK and is the reason it works.

The https://github.com/bluehazzard/cbMemoryView plugin currently cannot, without these changes request memory based on a symbol. Once these changes are incorporated the plugin can be enhanced to request memory via a symbol.

The GDB/MI debugger plugin already supports thee changes.

[ollydbg]

I have a question:

virtual uint64_t GetAddress() const = 0;

For a 32 bit system, does the address pointer need a 64bit variable?

[Andrew Cottrell]

No. The existing GDB code uses 64 bit or a string for address storage depending on where you look and how it is used.

An example of this is
`GDBMemoryRangeWatch(uint64_t address, uint64_t size, const wxString& symbol);
....
snip
....
uint64_t GetAddress() const { return m_address; }
uint64_t GetSize() const { return m_size; }

private:
    uint64_t m_address;
    uint64_t m_size;
    wxString m_symbol;
    wxString m_value;

};

[ollydbg]

OK, you are right.

BTW: The line 2251 of debuggergdb.cpp after applying your patch.

driver->SetMemoryRangeValue(addr, value);

should have an extra indent.

[Andrew Cottrell]

Is that the only change or modification etc you can see that needs to be done?
Any more queries?

Attached is an updated patch where I manually added the spaces to the patch file as it allot quicker than editing the file and creating the patch again.

[ollydbg]

I will take time to review this ticket this weekend.

[Andrew Cottrell]

Thanks if you take longer as it's Easter then let me know.

[ollydbg]

I reviewed and tested the patch v2, it works OK.
I will commit and push to the SVN repository soon, thanks!
BTW: I don't have Easter holiday here in China.

[ollydbg]

r12795

[Andrew Cottrell]

Thanks very much for looking and testing and applying the change, but I have no idea what could have gone wrong, but not all of the src/plugins/debuggergdb/debuggergdb.cpp changes in the patch file were applied for some strange reason. The first change went through, but the second did not...

The missing change is:
~~~
Index: src/plugins/debuggergdb/debuggergdb.cpp
===================================================================
--- src/plugins/debuggergdb/debuggergdb.cpp (revision 12783)
+++ src/plugins/debuggergdb/debuggergdb.cpp (working copy)
@@ -2243,11 +2242,20 @@
const WatchType type = itType->second;
if (type == WatchType::MemoryRange)
{

• DebuggerDriver* driver = m_State.GetDriver();
  cb::shared_ptr<gdbmemoryrangewatch> temp_watch = std::static_pointer_cast<gdbmemoryrangewatch>(watch);
  +
  uint64_t addr = temp_watch->GetAddress();
  -</gdbmemoryrangewatch></gdbmemoryrangewatch>
• DebuggerDriver* driver = m_State.GetDriver();
• if (addr == 0)
• {
  driver->SetMemoryRangeValue(addr, value);
• }
• else
• {
• wxString symbol;
• temp_watch->GetSymbol(symbol);
• driver->SetMemoryRangeValue(symbol, value);
• }
  }
  else
  {~~~

[ollydbg]

Oh, sorry about that. It looks like the patch command skip one chunk of your v2 patch.

$ patch -p1 < ./SF_CB_12783_GDB_Debugger_Changes_v2.patch
(Stripping trailing CRs from patch; use --binary to disable.)
patching file include/cbplugin.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file include/debuggermanager.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/cdb_driver.cpp
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/cdb_driver.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/debugger_defs.cpp
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/debugger_defs.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/debuggerdriver.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/debuggergdb.cpp
Hunk #2 FAILED at 2242.
1 out of 2 hunks FAILED -- saving rejects to file plugins/debuggergdb/debuggergdb.cpp.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/gdb_commands.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/gdb_driver.cpp
(Stripping trailing CRs from patch; use --binary to disable.)
patching file plugins/debuggergdb/gdb_driver.h

I will make a fix soon.

[ollydbg]

The missing patch hunk is applied in Committed r12796.

BTW, I'm not quite sure how the "watch range variable" (or a better name, watch an array)works. I just add a variable to the watch, and in the context menu, I can open the property dialog, and set the start and number value(this may regard the value as an array). That's all I can test.

Can you give me the steps to do the "watch ranges"? Thanks.

[Andrew Cottrell]

There are a number of inconsistencies and names that make it not intuitive. I got confused a number of times in the first few days of looking at the code until I got the hang of the names used and what they meant.

The range in the code changes is not a range , but a length that the memory watch uses for the length to show/return after the address or address of the symbol that is used as the start of the memory you want start at. In GDB and in the C::B memory dump or in the future with the cbMemoryView plugin you can specify either the memory address you want to start looking as as either a read address or a symbol.
included the small snippet required for the memory dump testing).
To test it you will need a variable that is a structure or array. In the example below I use cTest as one of the tests (in my file I have other code for testing GDB/MI, but I have only

To do the test compile the app, set a line break point on one of the strcat lines and then:
a) Open the menu "Debug->Debugging windows->memory dump" to open the memory dump dialog
b) In the memory dump dialog enter the following:
Address: &cTest
Bytes: 256
c) Step and check the memory ascii data has been updated to add the new strcat text or look at the physical memory hex values.
d) Change the address to the address shown in the memory dump output and step again to show that the address works.

You can also watch the cTest variable and in the watch window right click on the cTest and select the "examine memory" as it then brings up the memory dump dialog and puts the &cTest into the address field.

int main()
{
    char cTest[300];
    memset(cTest, 0x00, 300);
    //              00000000001111111111
    //              01234567890123456789
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "This is a char array");
    strcat(cTest, "END END END END END ");

return 0;
}