Last Comment Bug 739800 - Format profiler address tags with "0x" prefix
: Format profiler address tags with "0x" prefix
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Gecko Profiler (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla14
Assigned To: Vladan Djeric (:vladan)
:
:
Mentors:
Depends on: 741242
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-27 14:54 PDT by Vladan Djeric (:vladan)
Modified: 2012-04-01 14:20 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Force all l-tag addresses to have a "0x" prefix (900 bytes, patch)
2012-03-27 14:54 PDT, Vladan Djeric (:vladan)
b56girard: review+
vladan.bugzilla: checkin+
Details | Diff | Splinter Review

Description User image Vladan Djeric (:vladan) 2012-03-27 14:54:18 PDT
Created attachment 609901 [details] [diff] [review]
Force all l-tag addresses to have a "0x" prefix

The profiler uses different formats for l-tag addresses on different platforms. I suspect this is probably because of some global localization setting.

This patch forces all addresses to have a "0x" prefix. Surprisingly, specifying stringstream flags was not effective.
Comment 1 User image Benoit Girard (:BenWa) 2012-03-27 14:57:39 PDT
Comment on attachment 609901 [details] [diff] [review]
Force all l-tag addresses to have a "0x" prefix

This change should include a comment explaining the problem we saw on windows.
Comment 2 User image Markus Stange [:mstange] (away until Feb 22) 2012-03-27 16:11:26 PDT
Comment on attachment 609901 [details] [diff] [review]
Force all l-tag addresses to have a "0x" prefix

Does this work?

    stream << entry.mTagName << "-0x" << std::hex << reinterpret_cast<uintptr_t>(entry.mTagData) << "\n";
Comment 3 User image Benoit Girard (:BenWa) 2012-03-27 16:15:34 PDT
We had tried that on windows. It works in a test app but not in firefox.
Comment 4 User image Benoit Girard (:BenWa) 2012-03-27 16:16:16 PDT
Err nevermind that comments, I misread. Perhaps the above would work everywhere.
Comment 5 User image Vladan Djeric (:vladan) 2012-03-27 16:23:56 PDT
(In reply to Markus Stange from comment #2)
> Comment on attachment 609901 [details] [diff] [review]
> Force all l-tag addresses to have a "0x" prefix
> 
> Does this work?
> 
>     stream << entry.mTagName << "-0x" << std::hex <<
> reinterpret_cast<uintptr_t>(entry.mTagData) << "\n";

Yes, this would probably work although I think there might still be a small chance for inconsistency across environments since this format does not specify whether the hex alphabetic characters should be uppercase or lowercase.
Comment 6 User image Markus Stange [:mstange] (away until Feb 22) 2012-03-27 16:52:06 PDT
(In reply to Vladan Djeric (:vladan) from comment #5)
> there might still be a small
> chance for inconsistency across environments since this format does not
> specify whether the hex alphabetic characters should be uppercase or
> lowercase.

The C++ spec draft at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3092.pdf says in table 84 on page 685 to use lowercase characters (%x) as long as the "uppercase" flag isn't set (as you'd do with cout << hex << uppercase << 10, for example). Looks safe to me.
Comment 7 User image Vladan Djeric (:vladan) 2012-03-27 17:24:21 PDT
I a(In reply to Markus Stange from comment #6)
> (In reply to Vladan Djeric (:vladan) from comment #5)
> > there might still be a small
> > chance for inconsistency across environments since this format does not
> > specify whether the hex alphabetic characters should be uppercase or
> > lowercase.
> 
> The C++ spec draft at
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3092.pdf says in
> table 84 on page 685 to use lowercase characters (%x) as long as the
> "uppercase" flag isn't set (as you'd do with cout << hex << https://vdjeric@github.com/vdjeric/Gecko-Profiler-Addon.git<< 10,
> for example). Looks safe to me.

I agree that makes sense and it's what we first tried but for some reason we are seeing stringstream ignore our format specifiers on Windows Firefox but not in test apps.

It ignored setting "profile.flags(profile.flags() | std::ios::showbase)" as well as "profile << std::hex << std::showbase << ...", which is why i'm a little paranoid about trusting it to honor any string stream formatters.

Also, I just tried your suggestion and it seems to always format the hex addresses with "uppercase" and completely ignores any "nouppercase" format specifier :(
Comment 8 User image Markus Stange [:mstange] (away until Feb 22) 2012-03-27 17:52:31 PDT
Okay, strange. Then go ahead :)
Comment 10 User image Matt Brubeck (:mbrubeck) 2012-03-29 08:52:38 PDT
https://hg.mozilla.org/mozilla-central/rev/520ca2f9b4b7

Note You need to log in before you can comment on or make changes to this bug.