When we hit a breakpoint, there may be several breakpoints set at the same location. The location has a single `BreakpointSite`, and that has a doubly-linked list of `Breakpoint`s set at that site, each with its own handler. So the first thing we do when a breakpoint is hit is to gather up all the breakpoints at that location, as a vector of `Breakpoint*` pointers. Then, we iterate over that vector, and call each breakpoint's handler. But handlers are arbitrary JS, and one handler can remove another. To avoid firing handlers for breakpoints that have been removed, each iteration of the loop checks that the `Breakpoint*` it got from the vector is still present in the `BreakpointSite`'s linked list; if it's not found there, it's ignored. Unfortunately, removing a breakpoint from a site frees its `Breakpoint` structure, so the pointer in the vector becomes a dangling pointer. Comparing the dangling pointer to the elements of the `BreakpointSite`'s linked list is not UB (since C++ 14), but it is implementation-specified, and in our case clearly meaningless. If another `Breakpoint` happens to get allocated at the same address and inserted at the same site, we didn't intend to run it.
Bug 1589906 Comment 0 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
If several breakpoints are set at the same location, and one's handler removes the other, and then a third new breakpoint is set at that location, and the third breakpoint's underlying `js::Breakpoint` structure happens to get allocated at the same address as the deleted breakpoint's, then we will execute undefined behavior. In practice, it will almost certainly run the newly inserted breakpoint. So this is a minor problem. How it happens: When we hit a breakpoint, there may be several breakpoints set at the same location. The location has a single `BreakpointSite`, and that has a doubly-linked list of `Breakpoint`s set at that site, each with its own handler. So the first thing we do when a breakpoint is hit is to gather up all the breakpoints at that location, as a vector of `Breakpoint*` pointers. Then, we iterate over that vector, and call each breakpoint's handler. But handlers are arbitrary JS, and one handler can remove another. To avoid firing handlers for breakpoints that have been removed, each iteration of the loop checks that the `Breakpoint*` it got from the vector is still present in the `BreakpointSite`'s linked list; if it's not found there, it's ignored. Unfortunately, removing a breakpoint from a site frees its `Breakpoint` structure, so the pointer in the vector becomes a dangling pointer. Comparing the dangling pointer to the elements of the `BreakpointSite`'s linked list is not UB (since C++ 14), but then using it if a match is found *is* UB: just because a dead pointer compares equal to a live pointer doesn't mean that dereferencing the dead pointer is okay. So if another `Breakpoint` happens to get allocated at the same address and inserted at the same site, we get UB.
If several breakpoints are set at the same location, and one's handler removes the other, and then a third new breakpoint is set at that location, and the third breakpoint's underlying `js::Breakpoint` structure happens to get allocated at the same address as the deleted breakpoint's, then we will execute undefined behavior. In practice, it will almost certainly just run the newly inserted breakpoint. So this is a minor problem. How it happens: When we hit a breakpoint, there may be several breakpoints set at the same location. The location has a single `BreakpointSite`, and that has a doubly-linked list of `Breakpoint`s set at that site, each with its own handler. So the first thing we do when a breakpoint is hit is to gather up all the breakpoints at that location, as a vector of `Breakpoint*` pointers. Then, we iterate over that vector, and call each breakpoint's handler. But handlers are arbitrary JS, and one handler can remove another. To avoid firing handlers for breakpoints that have been removed, each iteration of the loop checks that the `Breakpoint*` it got from the vector is still present in the `BreakpointSite`'s linked list; if it's not found there, it's ignored. Unfortunately, removing a breakpoint from a site frees its `Breakpoint` structure, so the pointer in the vector becomes a dangling pointer. Comparing the dangling pointer to the elements of the `BreakpointSite`'s linked list is not UB (since C++ 14), but then using it if a match is found *is* UB: just because a dead pointer compares equal to a live pointer doesn't mean that dereferencing the dead pointer is okay. So if another `Breakpoint` happens to get allocated at the same address and inserted at the same site, we get UB.
If several breakpoints are set at the same location, and one's handler removes the other, and then a third new breakpoint is set at that location, and the third breakpoint's underlying `js::Breakpoint` structure happens to get allocated at the same address as the deleted breakpoint's, then we will execute undefined behavior. How it happens: When we hit a breakpoint, there may be several breakpoints set at the same location. The location has a single `BreakpointSite`, and that has a doubly-linked list of `Breakpoint`s set at that site, each with its own handler. So the first thing we do when a breakpoint is hit is to gather up all the breakpoints at that location, as a vector of `Breakpoint*` pointers. Then, we iterate over that vector, and call each breakpoint's handler. But handlers are arbitrary JS, and one handler can remove another. To avoid firing handlers for breakpoints that have been removed, each iteration of the loop checks that the `Breakpoint*` it got from the vector is still present in the `BreakpointSite`'s linked list; if it's not found there, it's ignored. Unfortunately, removing a breakpoint from a site frees its `Breakpoint` structure, so the pointer in the vector becomes a dangling pointer. Comparing the dangling pointer to the elements of the `BreakpointSite`'s linked list is not UB (since C++ 14), but then using it if a match is found *is* UB: just because a dead pointer compares equal to a live pointer doesn't mean that dereferencing the dead pointer is okay. So if another `Breakpoint` happens to get allocated at the same address and inserted at the same site, we get UB.