[llvm-bugs] [Bug 35975] New: Line-number/machine-code offsets break "set next statement" on Windows
via llvm-bugs
llvm-bugs at lists.llvm.org
Tue Jan 16 13:46:10 PST 2018
https://bugs.llvm.org/show_bug.cgi?id=35975
Bug ID: 35975
Summary: Line-number/machine-code offsets break "set next
statement" on Windows
Product: clang
Version: unspecified
Hardware: PC
OS: Windows NT
Status: NEW
Severity: enhancement
Priority: P
Component: LLVM Codegen
Assignee: unassignedclangbugs at nondot.org
Reporter: brucedawson at chromium.org
CC: llvm-bugs at lists.llvm.org
A Chromium developer reported that aggressive use of the VC++ debugger command
Set Next Statement worked very reliably with VC++ compiled binaries but
frequently causes crashes with clang-cl compiled binaries. The tracking
Chromium bug is crbug.com/793819.
Specifically, the debug information is generated such that invoking Set Next
Statement for a particular line of C++ code causes the assembly language
instruction pointer to be set to the *third* instruction used to implement that
line of code, instead of the *first* instruction. That means that two
instructions of setup are skipped, which leads to unpredictable results.
As a simple repro, run debug browser_tests.exe --single_process
--gtest_filter=CaptivePortalBrowserTest.Login
and then in this code:
"SetUpCaptivePortalService(browser()->profile(),
GURL(kMockCaptivePortalTestUrl));"
add a breakpoint. When it gets hit, you can move the statement somewhere else
(like repeat the EXPECT_EQ call) and then move it again to the
SetUpCaptivePortalService call. It'll crash in the std::string constructor.
The relevant source code is here:
// Double-check that the captive portal service isn't enabled by default for
// browser tests.
EXPECT_EQ(CaptivePortalService::DISABLED_FOR_TESTING,
CaptivePortalService::get_state_for_testing());
CaptivePortalService::set_state_for_testing(
CaptivePortalService::SKIP_OS_CHECK_FOR_TESTING);
EnableCaptivePortalDetection(browser()->profile(), true);
// Set the captive portal service to use URLRequestMockCaptivePortalJob's
// mock URL, by default.
SetUpCaptivePortalService(browser()->profile(),
GURL(kMockCaptivePortalTestUrl));
The machine code around the SetUpCaptivePortalService call is here:
0285668C E8 3F 02 00 00 call
CaptivePortalBrowserTest::EnableCaptivePortalDetection (028568D0h)
02856691 8D 4E 5C lea ecx,[esi+5Ch]
02856694 8D 05 5B FC 5C 00 lea eax,[string
"http://mock.captive.portal.test/"... (05CFC5Bh)]
0285669A 83 EC 04 sub esp,4
0285669D 89 04 24 mov dword ptr [esp],eax
028566A0 E8 7B 72 61 FE call
base::BasicStringPiece<...>::BasicStringPiece (0E6D920h)
028566A5 8D 8E 94 00 00 00 lea ecx,[esi+94h]
028566AB 8B 56 5C mov edx,dword ptr [esi+5Ch]
028566AE 8B 7E 60 mov edi,dword ptr [esi+60h]
028566B1 83 EC 08 sub esp,8
028566B4 89 14 24 mov dword ptr [esp],edx
028566B7 89 7C 24 04 mov dword ptr [esp+4],edi
028566BB 89 46 10 mov dword ptr [esi+10h],eax
028566BE FF 15 A8 28 DF 0A call dword ptr [__imp_GURL::GURL
(0ADF28A8h)]
028566C4 8B 4E 40 mov ecx,dword ptr [esi+40h]
028566C7 89 46 0C mov dword ptr [esi+0Ch],eax
028566CA E8 91 73 61 FE call InProcessBrowserTest::browser
(0E6DA60h)
028566CF 89 C1 mov ecx,eax
028566D1 E8 AA 73 61 FE call Browser::profile (0E6DA80h)
028566D6 8D 8E 94 00 00 00 lea ecx,[esi+94h]
028566DC 83 EC 08 sub esp,8
028566DF 8B 56 40 mov edx,dword ptr [esi+40h]
028566E2 89 4E 08 mov dword ptr [esi+8],ecx
028566E5 89 D1 mov ecx,edx
028566E7 89 04 24 mov dword ptr [esp],eax
028566EA 8B 46 08 mov eax,dword ptr [esi+8]
028566ED 89 44 24 04 mov dword ptr [esp+4],eax
028566F1 E8 6A 02 00 00 call
CaptivePortalBrowserTest::SetUpCaptivePortalService (02856960h)
Crucially, if you set a breakpoint on the call to SetUpCaptivePortalService or
if you Set Next Statement to that line of code then you get dropped onto
0285669A. That means that the stack gets adjusted properly (which is good) but
it means that ECX and EAX are not set up. Those are set up starting on 02856691
and if you Set Next Statement from EXPECT_EQ to SetUpCaptivePortalService then
they have the wrong values. They are supposed to be:
EAX = 005CFC5B ECX = 0019D89C
but they are actually:
EAX = 00000000 ECX = 0019D8B4
EAX and ECX are supposed to hold the address of the string used for the
constructor's input and the address of the object. These seem like important
things.
The bad value of EAX means that the string will be treated as a NULL string
instead of "http://mock.captive.portal.test/" and the incorrect value for ECX
turns into a stack tromp - stack corruption - which can only end in tears.
It looks like the problem would be avoided if the debug information for that
line was offset by two instructions.
Debugging techniques like this make me nervous but it is awesome if they work
in VS and we should try to support them as well as VS does.
My gn args are below. The top four are probably important (fastlink was causing
me problems so I used lld) and the bottom six are less so, but will help with
making the builds fast.
use_lld = true
is_debug = true
is_component_build = true
target_cpu = "x86"
enable_nacl = false
remove_webcore_debug_symbols=true
use_jumbo_build = true
goma_dir = "C:\src\goma\goma-win64"
use_goma = true
symbol_level = 2
--
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20180116/b52ef55a/attachment.html>
More information about the llvm-bugs
mailing list