)]}'
{
  "commit": "1b28c0f488bb84c8cc9f680afa9680dc0b0e2b3d",
  "tree": "66ed9fdd1e445b56cd6b2faa35b30cd94f7de181",
  "parents": [
    "e5b3bfa519a9704288219d09fef1a792663351c0"
  ],
  "author": {
    "name": "Andrew Burgess",
    "email": "aburgess@redhat.com",
    "time": "Tue Oct 03 17:20:14 2023 +0100"
  },
  "committer": {
    "name": "Simon Marchi",
    "email": "simon.marchi@efficios.com",
    "time": "Thu Oct 05 13:20:45 2023 -0400"
  },
  "message": "gdb: fix auxv cache clearing from new_objfile observer\n\nIt was pointed out on the mailing list that a recently added\ntest (gdb.python/py-progspace-events.exp) was failing when run with\nthe native-extended-gdbserver board.  This test was added with this\ncommit:\n\n  commit 59912fb2d22f8a4bb0862487f12a5cc65b6a013f\n  Date:   Tue Sep 19 11:45:36 2023 +0100\n\n      gdb: add Python events for program space addition and removal\n\nIt turns out though that the test is failing due to a existing bug\nin GDB, the new test just exposes the problem.  Additionally, the\nfailure really doesn\u0027t even rely on the new functionality added in the\nabove commit.  I reduced the test to a simple set of steps that\nreproduced the failure and tested against GDB 13, and the test passes;\nso the bug was introduced since then.  In fact, the bug was introduced\nwith this commit:\n\n  commit a2827364e2bf19910fa5a54364a594a5ba3033b8\n  Date:   Fri Sep 8 15:48:16 2023 +0100\n\n      gdb: remove final user of the executable_changed observer\n\nThis commit changed how the per-inferior auxv data cache is managed,\nspecifically, when the cache is cleared, and it is this that leads to\nthe failure.\n\nThis bug is interesting because it exposes a number of issues with\nGDB, I\u0027ll explain all of the problems I see, though ultimately, I only\npropose fixing one problem in this commit, which is enough to resolve\nthe crash we are currently seeing.\n\nThe crash that we are seeing manifests like this:\n\n  ...\n  [Inferior 2 (process 3970384) exited normally]\n  +inferior 1\n  [Switching to inferior 1 [process 3970383] (/tmp/build/gdb/testsuite/outputs/gdb.python/py-progspace-events/py-progspace-events)]\n  [Switching to thread 1.1 (Thread 3970383.3970383)]\n  #0  breakpt () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.python/py-progspace-events.c:28\n  28\t{ /* Nothing.  */ }\n  (gdb) step\n  +step\n  terminate called after throwing an instance of \u0027gdb_exception_error\u0027\n\n  Fatal signal: Aborted\n  ... etc ...\n\nWhat\u0027s happening is that GDB attempts to refill the auxv cache as a\nresult of the gdbarch_has_shared_address_space call in\nprogram_space::~program_space, the backtrace looks like this:\n\n  #0  0x00007fb4f419a9a5 in raise () from /lib64/libpthread.so.0\n  #1  0x00000000008b635d in handle_fatal_signal (sig\u003d6) at ../../src/gdb/event-top.c:912\n  #2  \u003csignal handler called\u003e\n  #3  0x00007fb4f38e3625 in raise () from /lib64/libc.so.6\n  #4  0x00007fb4f38cc8d9 in abort () from /lib64/libc.so.6\n  #5  0x00007fb4f3c70756 in __gnu_cxx::__verbose_terminate_handler() [clone .cold] () from /lib64/libstdc++.so.6\n  #6  0x00007fb4f3c7c6dc in __cxxabiv1::__terminate(void (*)()) () from /lib64/libstdc++.so.6\n  #7  0x00007fb4f3c7b6e9 in __cxa_call_terminate () from /lib64/libstdc++.so.6\n  #8  0x00007fb4f3c7c094 in __gxx_personality_v0 () from /lib64/libstdc++.so.6\n  #9  0x00007fb4f3a80c63 in _Unwind_RaiseException_Phase2 () from /lib64/libgcc_s.so.1\n  #10 0x00007fb4f3a8154e in _Unwind_Resume () from /lib64/libgcc_s.so.1\n  #11 0x0000000000e8832d in target_read_alloc_1\u003cunsigned char\u003e (ops\u003d0x408a3a0, object\u003dTARGET_OBJECT_AUXV, annex\u003d0x0) at ../../src/gdb/target.c:2266\n  #12 0x0000000000e73dea in target_read_alloc (ops\u003d0x408a3a0, object\u003dTARGET_OBJECT_AUXV, annex\u003d0x0) at ../../src/gdb/target.c:2315\n  #13 0x000000000058248c in target_read_auxv_raw (ops\u003d0x408a3a0) at ../../src/gdb/auxv.c:379\n  #14 0x000000000058243d in target_read_auxv () at ../../src/gdb/auxv.c:368\n  #15 0x000000000058255c in target_auxv_search (match\u003d0x0, valp\u003d0x7ffdee17c598) at ../../src/gdb/auxv.c:415\n  #16 0x0000000000a464bb in linux_is_uclinux () at ../../src/gdb/linux-tdep.c:433\n  #17 0x0000000000a464f6 in linux_has_shared_address_space (gdbarch\u003d0x409a2d0) at ../../src/gdb/linux-tdep.c:440\n  #18 0x0000000000510eae in gdbarch_has_shared_address_space (gdbarch\u003d0x409a2d0) at ../../src/gdb/gdbarch.c:4889\n  #19 0x0000000000bc7558 in program_space::~program_space (this\u003d0x4544aa0, __in_chrg\u003d\u003coptimized out\u003e) at ../../src/gdb/progspace.c:124\n  #20 0x00000000009b245d in delete_inferior (inf\u003d0x47b3de0) at ../../src/gdb/inferior.c:290\n  #21 0x00000000009b2c10 in prune_inferiors () at ../../src/gdb/inferior.c:480\n  #22 0x00000000009c5e3e in fetch_inferior_event () at ../../src/gdb/infrun.c:4558\n  #23 0x000000000099b4dc in inferior_event_handler (event_type\u003dINF_REG_EVENT) at ../../src/gdb/inf-loop.c:42\n  #24 0x0000000000cbc64f in remote_async_serial_handler (scb\u003d0x4090a30, context\u003d0x408a6b0) at ../../src/gdb/remote.c:14859\n  #25 0x0000000000d83d3a in run_async_handler_and_reschedule (scb\u003d0x4090a30) at ../../src/gdb/ser-base.c:138\n  #26 0x0000000000d83e1f in fd_event (error\u003d0, context\u003d0x4090a30) at ../../src/gdb/ser-base.c:189\n\nSo this is problem #1, if we throw an exception while deleting a\nprogram_space then this is not caught, and is going to crash GDB.\n\nProblem #2 becomes evident when we ask why GDB is throwing an error in\nthis case; the error is thrown because the remote target, operating in\nnon-async mode, can\u0027t read the auxv data while an inferior is running\nand GDB is waiting for a stop reply.  The problem here then, is why\ndoes GDB get into a position where it tries to interact with the\nremote target in this way, at this time?  The problem is caused by the\nprune_inferiors call which can be seen in the above backtrace.\n\nIn prune_inferiors we check if the inferior is deletable, and if it\nis, we delete it.  The problem is, I think, we should also check if\nthe target is currently in a state that would allow us to delete the\ninferior.  We don\u0027t currently have such a check available, we\u0027d need\nto add one, but for the remote target, this would return false if the\nremote is in async mode and the remote is currently waiting for a stop\nreply.  With this change in place GDB would defer deleting the\ninferior until the remote target has stopped, at which point GDB would\nbe able to refill the auxv cache successfully.\n\nAnd then, problem #3 becomes evident when we ask why GDB is needing to\nrefill the auxv cache now when it didn\u0027t need to for GDB 13.  This is\nwhere the second commit mentioned above (a2827364e2bf) comes in.\nPrior to this commit, the auxv cache was cleared by the\nexecutable_changed observer, while after that commit the auxv cache\nwas cleared by the new_objfile observer -- but only when the\nnew_objfile observer is used in the special mode that actually means\nthat all objfiles have been unloaded (I know, the overloading of the\nnew_objfile observer is horrible, and unnecessary, but it\u0027s not really\nimportant for this bug).\n\nThe difference arises because the new_objfile observer is triggered\nfrom clear_symtab_users, which in turn is called from\nprogram_space::~program_space.  The new_objfile observer for auxv does\nthis:\n\n  static void\n  auxv_new_objfile_observer (struct objfile *objfile)\n  {\n    if (objfile \u003d\u003d nullptr)\n      invalidate_auxv_cache_inf (current_inferior ());\n  }\n\nThat is, when all the objfiles are unloaded, we clear the auxv cache\nfor the current inferior.\n\nThe problem is, then when we look at the prune_inferiors -\u003e\ndelete_inferior -\u003e ~program_space path, we see that the current\ninferior is not going to be an inferior that exists within the\nprogram_space being deleted; delete_inferior removes the deleted\ninferior from the global inferior list, and then only deletes the\nprogram_space if program_space::empty() returns true, which is only\nthe case if the current inferior isn\u0027t within the program_space to\ndelete, and no other inferior exists within that program_space\neither.\n\nWhat this means is that when the new_objfile observer is called we\ncan\u0027t rely on the current inferior having any relationship with the\nprogram space in which the objfiles were removed.  This was an error\nin the commit a2827364e2bf, the only thing we can rely on is the\ncurrent program space.  As a result of this mistake, after commit\na2827364e2bf, GDB was sometimes clearing the auxv cache for a random\ninferior.  In the native target case this was harmless as we can\nalways refill the cache when needed, but in the remote target case, if\nwe need to refill the cache when the remote target is executing, then\nwe get the crash we observed.\n\nAnd additionally, if we think about this a little more, we see that\ncommit a2827364e2bf made another mistake.  When all the objfiles are\nremoved, they are removed from a program_space, a program_space might\ncontain multiple inferiors, so surely, we should clear the auxv cache\nfor all of the matching inferiors?\n\nGiven these two insights, that the current_inferior is not relevant,\nonly the current_program_space, and that we should be clearing the\ncache for all inferiors in the current_program_space, we can update\nauxv_new_objfile_observer to:\n\n  if (objfile \u003d\u003d nullptr)\n    {\n      for (inferior *inf : all_inferiors ())\n\t{\n\t  if (inf-\u003epspace \u003d\u003d current_program_space)\n\t    invalidate_auxv_cache_inf (inf);\n\t}\n    }\n\nWith this change we now correctly clear the auxv cache for the correct\ninferiors, and GDB no longer needs to refill the cache at an\ninconvenient time, this avoids the crash we were seeing.\n\nAnd finally, we reach problem #4.  Inspired by the observation that\nusing the current_inferior from within the ~program_space function was\nnot correct, I added some debug to see if current_inferior() was\ncalled anywhere else (below ~program_space), and the answer is yes,\nit\u0027s called a often.  Mostly the culprit is GDB doing:\n\n  current_inferior ()-\u003etop_target ()-\u003e ....\n\nBut I think all of these calls are most likely doing the wrong thing,\nand only work because the top target in all these cases is shared\nbetween all inferiors, e.g. it\u0027s the native target, or the remote\ntarget for all inferiors.  But if we had a truly multi-connection\nsetup, then we might start to see odd behaviour.\n\nProblem #1 I\u0027m just ignoring for now, I guess at some point we might\nrun into this again, and then we\u0027d need to solve this.  But in this\ncase I wasn\u0027t sure what a \"good\" solution would look like.  We need\nthe auxv data in order to implement the linux_is_uclinux() function.\nIf we can\u0027t get the auxv data then what should we do, assume yes, or\nassume no?  The right answer would probably be to propagate the error\nback up the stack, but then we reach ~program_space, and throwing\nexceptions from a destructor is problematic, so we\u0027d need to catch and\ndeal at this point.  The linux_is_uclinux() call is made from within\ngdbarch_has_shared_address_space(), which is used like:\n\n  if (!gdbarch_has_shared_address_space (target_gdbarch ()))\n    delete this-\u003easpace;\n\nSo, we would have to choose; delete the address space or not.  If we\ndelete it on error, then we might delete an address space that is\nshared within another program space.  If we don\u0027t delete the address\nspace, then we might leak it.  Neither choice is great.\n\nA better solution might be to have the address spaces be reference\ncounted, then we could remove the gdbarch_has_shared_address_space\ncall completely, and just rely on the reference count to auto-delete\nthe address space when appropriate.\n\nThe solution for problem #2 I already hinted at above, we should have\na new target_can_delete_inferiors() call, which should be called from\nprune_inferiors, this would prevent GDB from trying to delete\ninferiors when a (remote) target is in a state where we know it can\u0027t\ndelete the inferior.  Deleting an inferior often (always?) requires\nsending packets to the remote, and if the remote is waiting for a stop\nreply then this will never work, so the pruning should be deferred in\nthis case.\n\nThe solution for problem #3 is included in this commit.\n\nAnd, for problem #4, I\u0027m not sure what the right solution is.  Maybe\ndelete_inferior should ensure the inferior to be deleted is in place\nwhen ~program_space is called?  But that seems a little weird, as the\ncurrent inferior would, in theory, still be using the current\nprogram_space...\n\nAnyway, after this commit, the gdb.python/py-progspace-events.exp test\nnow passes when run with the native-extended-remote board.\n\nBug: https://sourceware.org/bugzilla/show_bug.cgi?id\u003d30935\nApproved-By: Simon Marchi \u003csimon.marchi@efficios.com\u003e\nChange-Id: I41f0e6e2d7ecc1e5e55ec170f37acd4052f46eaf\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "9f599b04a4fe8679c0fb205df4d1b55a1889d5fd",
      "old_mode": 33188,
      "old_path": "gdb/auxv.c",
      "new_id": "cfb44ffa0bb7509197177533042a5f901e0ac8fb",
      "new_mode": 33188,
      "new_path": "gdb/auxv.c"
    }
  ]
}