Open Bug 1491241 Opened 6 years ago Updated 2 years ago

[clang, visual studio debugging] Local variables that are yet out of scope (defined later) are visible in Locals or show (wrong) value when hovered over

Categories

(Firefox Build System :: Toolchains, defect)

Product:
Firefox Build System
Component:
Toolchains
Version:
64 Branch
Platform:
x86_64
Windows 10
Type:
defect

Tracking

(Not tracked)

People

(Reporter: mayhemer, Unassigned)

References

Details

Attachments

(1 file)

Disable C5045 in security/cerverifier and security/pkix/test/gtest/
4.09 KB, patch
Details | Diff | Splinter Review 
STR:
- enable-debug and disable-optimize build with a patch from bug 1490743 (not should that would matter)
- put a breakpoint at [1], let hit
- hover over |thirdPartyUtil| local var defined later (now out of scope)

AR:
- shows LoadInfo object being contained, which is definitely wrong
- that variable is also visible in the Locals watch window

ER:
I'm not sure what cl build shows, but I believe out-of-scope vars are not in Locals and nothing happens on hover.  If added manually to Watch the result should be "out of scope" or "symbol not found" or something like this.


This is a minor issue, but in more complicated scenarios could be confusing.


[1] https://searchfox.org/mozilla-central/rev/dd965445ec47fbf3cee566eff93b301666bda0e1/toolkit/components/antitracking/AntiTrackingCommon.cpp#715
I didn't hit your breakpoint in a simple startup, but I set a breakpoint on the top of that function, and VS and clang-cl both show the not-yet-defined variables with garbage values. Are you sure this is a regression?
I believe it's a regressions.  Which recent changeset to update m-c to get a cl build?  I can try.
You can build it with `export CC=cl LINKER=link` in your mozconfig.
(In reply to David Major [:dmajor] from comment #3)
> You can build it with `export CC=cl LINKER=link` in your mozconfig.

Tried (also by reverting the enable-clang-by-default patch), the build doesn't finish.  Note that I have updated to the latest VS.
What error message did you see? You don't have to revert the enable-clang-by-default patch. I can build the tree using MSVC 2017 15.8.4.
I could reproduce the error:

 0:21.87 Unified_cpp_certverifier0.cpp
 0:21.87 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(162) : error C2220: 警告をエラーとして扱いました。'object' ファイルは生成されません。
 0:21.87 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(701) : error C2220: 警告をエラーとして扱いました。'object' ファイルは生成されません。
 0:21.88 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(162) : warning C5045: /Qspectre スイッチが
 0:21.88 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(161) : 注: この行で比較を行ってインデックス 'i' の 範囲を確認します
 0:21.88 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(178) : 注: この行に呼び出しを入力します に指定され ている場合、コンパイラはメモリの読み込みに Spectre の軽減策を挿入します
 0:21.88 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(701) : warning C5045: /Qspectre スイッチが
 0:21.88 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(693) : 注: この行で比較を行ってインデックス 'j' の 範囲を確認します
 0:21.88 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(701) : 注: この行に呼び出しを入力します に指定され ている場合、コンパイラはメモリの読み込みに Spectre の軽減策を挿入します
 0:21.89 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(612) : warning C5045: /Qspectre スイッチが
 0:21.90 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(604) : 注: この行で比較を行ってインデックス 'i' の 範囲を確認します
 0:21.90 e:\m\mozilla-unified\security\certverifier\certverifier.cpp(612) : 注: この行に呼び出しを入力します に指定され ている場合、コンパイラはメモリの読み込みに Spectre の軽減策を挿入します
 0:21.91 e:\m\mozilla-unified\security\certverifier\ocspcache.cpp(337) : warning C5045: /Qspectre スイッチが
 0:21.91 e:\m\mozilla-unified\security\certverifier\ocspcache.cpp(335) : 注: この行で比較を行ってインデックス 'entry' の範囲を確認します
 0:21.91 e:\m\mozilla-unified\security\certverifier\ocspcache.cpp(337) : 注: この行に呼び出しを入力します に指定されている場合、コンパイラはメモリの読み込みに Spectre の軽減策を挿入します
 0:21.91 mozmake.EXE[4]: *** [e:/m/mozilla-unified/config/rules.mk:1119: Unified_cpp_certverifier0.obj] Error 2
 0:21.91 mozmake.EXE[3]: *** [e:/m/mozilla-unified/config/recurse.mk:74: security/certverifier/target] Error 2
 0:23.43 CTLogVerifierTest.cpp
 0:23.80 CTObjectsExtractorTest.cpp
 0:24.11 CTTestUtils.cpp
 0:24.11 CTSerializationTest.cpp
 0:24.13 CTPolicyEnforcerTest.cpp
 0:24.81 MultiLogCTVerifierTest.cpp
 0:25.13 TrustOverrideTest.cpp
 0:25.15 mozmake.EXE[2]: *** [e:/m/mozilla-unified/config/recurse.mk:34: compile] Error 2
 0:25.15 mozmake.EXE[1]: *** [e:/m/mozilla-unified/config/rules.mk:432: default] Error 2
 0:25.15 mozmake.EXE: *** [client.mk:150: build] Error 2
 0:25.19 57 compiler warnings present.

We should add /wd5045 to HOST_CXXFALGS?
Although this patch will make MSVC builds work, I'm not sure we should just swallow the Spectre diagnostics.
Interesting, I didn't see any compile errors, but I have VS 15.8.3.
Hmm, we should really figure out why there are differences.
OK, I got C5045 only if I disabled the optimization (--disable-optimize). I think we can disable C5045 because we will never ship non-optimized builds.
Thank you for investigating. r=me if you want to land it, but it should be a separate bug as it's not related to the debug symbols in comment 0. (I should have asked for this earlier -- sorry for the distraction, mayhemer.)
Severity: minor → S4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: