<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
span.EmailStyle17
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;
        font-family:"Calibri",sans-serif;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">> Does this match how msvc emits S_LDATA32?<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Yes, the new behavior matches how Visual Studio 2017 emits S_LDATA32 symbols.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">                - Brock<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Zachary Turner [mailto:zturner@google.com]
<br>
<b>Sent:</b> Wednesday, December 5, 2018 3:27 PM<br>
<b>To:</b> reviews+D55336+public+f46083dda81dc3cf@reviews.llvm.org<br>
<b>Cc:</b> Wyma, Brock <brock.wyma@intel.com>; rnk@google.com; llvm-commits@lists.llvm.org; john.reagan@vmssoftware.com<br>
<b>Subject:</b> Re: [PATCH] D55336: [CodeView] Emit global variables within lexical scopes to limit visibility<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Does this match how msvc emits S_LDATA32?<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal">On Wed, Dec 5, 2018 at 12:15 PM Brock Wyma via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-bottom:12.0pt">bwyma created this revision.<br>
bwyma added reviewers: rnk, zturner.<br>
<br>
Currently, global variables with local visibility such as the following...<br>
<br>
  void foo() {<br>
    static int local = 1;<br>
  }<br>
<br>
  void bar() {<br>
    static int local = 3;<br>
  }<br>
<br>
...  are emitted in CodeView as global S_LDATA32 entries outside of any routine:<br>
<br>
  CodeViewDebugInfo [<br>
    Section: .debug$S (5)<br>
    Subsection [<br>
      GlobalProcIdSym {<br>
        Kind: S_GPROC32_ID (0x1147)<br>
        DisplayName: foo                              # Routine foo()<br>
      }<br>
      ProcEnd {<br>
        Kind: S_PROC_ID_END (0x114F)<br>
      }<br>
    ]<br>
    Subsection [<br>
      GlobalProcIdSym {<br>
        Kind: S_GPROC32_ID (0x1147)<br>
        DisplayName: bar                              # Routine bar()<br>
      }<br>
      ProcEnd {<br>
        Kind: S_PROC_ID_END (0x114F)<br>
      }<br>
    ]<br>
    Subsection [<br>
      DataSym {<br>
        Kind: S_LDATA32 (0x110C)<br>
        DisplayName: local                            # "local" inside "foo"<br>
        LinkageName: ?local@?1??foo@@YAXXZ@4HA<br>
      }<br>
      DataSym {<br>
        Kind: S_LDATA32 (0x110C)<br>
        DisplayName: local                            # "local" inside "bar"<br>
        LinkageName: ?local@?1??bar@@YAXXZ@4HA<br>
      }<br>
    ]<br>
  ]<br>
<br>
When debugging routines foo() and bar(), Visual Studio 2017 will locate whichever value for "local" it finds first, which may not be the correct one, and happily display the potentially incorrect value.<br>
<br>
This change builds upon the lexical block support added in rL327620 <<a href="https://reviews.llvm.org/rL327620" target="_blank">https://reviews.llvm.org/rL327620</a>> to sort global variables according to lexical scope. The result for the example above after
 this patch looks like this:<br>
<br>
  CodeViewDebugInfo [<br>
    Section: .debug$S (5)<br>
    Subsection [<br>
      SubSectionType: Symbols (0xF1)<br>
      GlobalProcIdSym {<br>
        Kind: S_GPROC32_ID (0x1147)<br>
        DisplayName: foo                              # Routine foo()<br>
      }<br>
      DataSym {<br>
        Kind: S_LDATA32 (0x110C)<br>
        DisplayName: local                            # "local" inside "foo"<br>
        LinkageName: ?local@?1??foo@@YAXXZ@4HA<br>
      }<br>
      ProcEnd {<br>
        Kind: S_PROC_ID_END (0x114F)<br>
      }<br>
    ]<br>
    Subsection [<br>
      SubSectionType: Symbols (0xF1)<br>
      GlobalProcIdSym {<br>
        Kind: S_GPROC32_ID (0x1147)<br>
        DisplayName: bar                              # Routine bar()<br>
      }<br>
      DataSym {<br>
        Kind: S_LDATA32 (0x110C)<br>
        DisplayName: local                            # "local" inside "bar"<br>
        LinkageName: ?local@?1??bar@@YAXXZ@4HA<br>
      }<br>
      ProcEnd {<br>
        Kind: S_PROC_ID_END (0x114F)<br>
      }<br>
    ]<br>
  ]<br>
<br>
The changes to the test type-quals.ll are index adjustments because of a minor change in the order in which the entries appear. I added the test global_visibility.ll to validate the new scoping behavior.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D55336" target="_blank">https://reviews.llvm.org/D55336</a><br>
<br>
Files:<br>
  lib/CodeGen/AsmPrinter/CodeViewDebug.cpp<br>
  lib/CodeGen/AsmPrinter/CodeViewDebug.h<br>
  test/DebugInfo/COFF/global_visibility.ll<br>
  test/DebugInfo/COFF/type-quals.ll<o:p></o:p></p>
</blockquote>
</div>
</div>
</body>
</html>