[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/7] x86/wait: prevent duplicated assembly labels


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 14 Mar 2025 09:05:07 +0000
  • Autocrypt: addr=andrew.cooper3@xxxxxxxxxx; keydata= xsFNBFLhNn8BEADVhE+Hb8i0GV6mihnnr/uiQQdPF8kUoFzCOPXkf7jQ5sLYeJa0cQi6Penp VtiFYznTairnVsN5J+ujSTIb+OlMSJUWV4opS7WVNnxHbFTPYZVQ3erv7NKc2iVizCRZ2Kxn srM1oPXWRic8BIAdYOKOloF2300SL/bIpeD+x7h3w9B/qez7nOin5NzkxgFoaUeIal12pXSR Q354FKFoy6Vh96gc4VRqte3jw8mPuJQpfws+Pb+swvSf/i1q1+1I4jsRQQh2m6OTADHIqg2E ofTYAEh7R5HfPx0EXoEDMdRjOeKn8+vvkAwhviWXTHlG3R1QkbE5M/oywnZ83udJmi+lxjJ5 YhQ5IzomvJ16H0Bq+TLyVLO/VRksp1VR9HxCzItLNCS8PdpYYz5TC204ViycobYU65WMpzWe LFAGn8jSS25XIpqv0Y9k87dLbctKKA14Ifw2kq5OIVu2FuX+3i446JOa2vpCI9GcjCzi3oHV e00bzYiHMIl0FICrNJU0Kjho8pdo0m2uxkn6SYEpogAy9pnatUlO+erL4LqFUO7GXSdBRbw5 gNt25XTLdSFuZtMxkY3tq8MFss5QnjhehCVPEpE6y9ZjI4XB8ad1G4oBHVGK5LMsvg22PfMJ ISWFSHoF/B5+lHkCKWkFxZ0gZn33ju5n6/FOdEx4B8cMJt+cWwARAQABzSlBbmRyZXcgQ29v cGVyIDxhbmRyZXcuY29vcGVyM0BjaXRyaXguY29tPsLBegQTAQgAJAIbAwULCQgHAwUVCgkI CwUWAgMBAAIeAQIXgAUCWKD95wIZAQAKCRBlw/kGpdefoHbdD/9AIoR3k6fKl+RFiFpyAhvO 59ttDFI7nIAnlYngev2XUR3acFElJATHSDO0ju+hqWqAb8kVijXLops0gOfqt3VPZq9cuHlh IMDquatGLzAadfFx2eQYIYT+FYuMoPZy/aTUazmJIDVxP7L383grjIkn+7tAv+qeDfE+txL4 SAm1UHNvmdfgL2/lcmL3xRh7sub3nJilM93RWX1Pe5LBSDXO45uzCGEdst6uSlzYR/MEr+5Z JQQ32JV64zwvf/aKaagSQSQMYNX9JFgfZ3TKWC1KJQbX5ssoX/5hNLqxMcZV3TN7kU8I3kjK mPec9+1nECOjjJSO/h4P0sBZyIUGfguwzhEeGf4sMCuSEM4xjCnwiBwftR17sr0spYcOpqET ZGcAmyYcNjy6CYadNCnfR40vhhWuCfNCBzWnUW0lFoo12wb0YnzoOLjvfD6OL3JjIUJNOmJy RCsJ5IA/Iz33RhSVRmROu+TztwuThClw63g7+hoyewv7BemKyuU6FTVhjjW+XUWmS/FzknSi dAG+insr0746cTPpSkGl3KAXeWDGJzve7/SBBfyznWCMGaf8E2P1oOdIZRxHgWj0zNr1+ooF /PzgLPiCI4OMUttTlEKChgbUTQ+5o0P080JojqfXwbPAyumbaYcQNiH1/xYbJdOFSiBv9rpt TQTBLzDKXok86M7BTQRS4TZ/ARAAkgqudHsp+hd82UVkvgnlqZjzz2vyrYfz7bkPtXaGb9H4 Rfo7mQsEQavEBdWWjbga6eMnDqtu+FC+qeTGYebToxEyp2lKDSoAsvt8w82tIlP/EbmRbDVn 7bhjBlfRcFjVYw8uVDPptT0TV47vpoCVkTwcyb6OltJrvg/QzV9f07DJswuda1JH3/qvYu0p vjPnYvCq4NsqY2XSdAJ02HrdYPFtNyPEntu1n1KK+gJrstjtw7KsZ4ygXYrsm/oCBiVW/OgU g/XIlGErkrxe4vQvJyVwg6YH653YTX5hLLUEL1NS4TCo47RP+wi6y+TnuAL36UtK/uFyEuPy wwrDVcC4cIFhYSfsO0BumEI65yu7a8aHbGfq2lW251UcoU48Z27ZUUZd2Dr6O/n8poQHbaTd 6bJJSjzGGHZVbRP9UQ3lkmkmc0+XCHmj5WhwNNYjgbbmML7y0fsJT5RgvefAIFfHBg7fTY/i kBEimoUsTEQz+N4hbKwo1hULfVxDJStE4sbPhjbsPCrlXf6W9CxSyQ0qmZ2bXsLQYRj2xqd1 bpA+1o1j2N4/au1R/uSiUFjewJdT/LX1EklKDcQwpk06Af/N7VZtSfEJeRV04unbsKVXWZAk uAJyDDKN99ziC0Wz5kcPyVD1HNf8bgaqGDzrv3TfYjwqayRFcMf7xJaL9xXedMcAEQEAAcLB XwQYAQgACQUCUuE2fwIbDAAKCRBlw/kGpdefoG4XEACD1Qf/er8EA7g23HMxYWd3FXHThrVQ HgiGdk5Yh632vjOm9L4sd/GCEACVQKjsu98e8o3ysitFlznEns5EAAXEbITrgKWXDDUWGYxd pnjj2u+GkVdsOAGk0kxczX6s+VRBhpbBI2PWnOsRJgU2n10PZ3mZD4Xu9kU2IXYmuW+e5KCA vTArRUdCrAtIa1k01sPipPPw6dfxx2e5asy21YOytzxuWFfJTGnVxZZSCyLUO83sh6OZhJkk b9rxL9wPmpN/t2IPaEKoAc0FTQZS36wAMOXkBh24PQ9gaLJvfPKpNzGD8XWR5HHF0NLIJhgg 4ZlEXQ2fVp3XrtocHqhu4UZR4koCijgB8sB7Tb0GCpwK+C4UePdFLfhKyRdSXuvY3AHJd4CP 4JzW0Bzq/WXY3XMOzUTYApGQpnUpdOmuQSfpV9MQO+/jo7r6yPbxT7CwRS5dcQPzUiuHLK9i nvjREdh84qycnx0/6dDroYhp0DFv4udxuAvt1h4wGwTPRQZerSm4xaYegEFusyhbZrI0U9tJ B8WrhBLXDiYlyJT6zOV2yZFuW47VrLsjYnHwn27hmxTC/7tvG3euCklmkn9Sl9IAKFu29RSo d5bD8kMSCYsTqtTfT6W4A3qHGvIDta3ptLYpIAOD2sY3GYq2nf3Bbzx81wZK14JdDDHUX2Rs 6+ahAA==
  • Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 14 Mar 2025 09:05:16 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 14/03/2025 8:44 am, Jan Beulich wrote:
> On 14.03.2025 09:30, Roger Pau Monné wrote:
>> On Fri, Mar 14, 2025 at 09:24:09AM +0100, Jan Beulich wrote:
>>> On 13.03.2025 16:30, Roger Pau Monne wrote:
>>>> When enabling UBSAN with clang, the following error is triggered during the
>>>> build:
>>>>
>>>> common/wait.c:154:9: error: symbol '.L_wq_resume' is already defined
>>>>   154 |         "push %%rbx; push %%rbp; push %%r12;"
>>>>       |         ^
>>>> <inline asm>:1:121: note: instantiated into assembly here
>>>>     1 |         push %rbx; push %rbp; push %r12;push %r13; push %r14; push 
>>>> %r15;sub %esp,%ecx;cmp $4096, %ecx;ja .L_skip;mov %rsp,%rsi;.L_wq_resume: 
>>>> rep movsb;mov %rsp,%rsi;.L_skip:pop %r15; pop %r14; pop %r13;pop %r12; pop 
>>>> %rbp; pop %rbx
>>>>       |                                                                    
>>>>                                                             ^
>>>> common/wait.c:154:9: error: symbol '.L_skip' is already defined
>>>>   154 |         "push %%rbx; push %%rbp; push %%r12;"
>>>>       |         ^
>>>> <inline asm>:1:159: note: instantiated into assembly here
>>>>     1 |         push %rbx; push %rbp; push %r12;push %r13; push %r14; push 
>>>> %r15;sub %esp,%ecx;cmp $4096, %ecx;ja .L_skip;mov %rsp,%rsi;.L_wq_resume: 
>>>> rep movsb;mov %rsp,%rsi;.L_skip:pop %r15; pop %r14; pop %r13;pop %r12; pop 
>>>> %rbp; pop %rbx
>>>>       |                                                                    
>>>>                                                                            
>>>>                        ^
>>>> 2 errors generated.
>>>>
>>>> The inline assembly block in __prepare_to_wait() is duplicated, thus
>>>> leading to multiple definitions of the otherwise unique labels inside the
>>>> assembly block.  GCC extended-asm documentation notes the possibility of
>>>> duplicating asm blocks:
>>>>
>>>>> Under certain circumstances, GCC may duplicate (or remove duplicates of)
>>>>> your assembly code when optimizing. This can lead to unexpected duplicate
>>>>> symbol errors during compilation if your asm code defines symbols or
>>>>> labels. Using ‘%=’ (see AssemblerTemplate) may help resolve this problem.
>>>> Move the assembly blocks that deal with saving and restoring the current
>>>> CPU context into it's own explicitly non-inline functions.  This prevents
>>>> clang from duplicating the assembly blocks.  Just using noinline attribute
>>>> seems to be enough to prevent assembly duplication, in the future noclone
>>>> might also be required if asm block duplication issues arise again.
>>> Wouldn't it be a far easier / less intrusive change to simply append %= to
>>> the label names?
>> That won't work AFAICT, as the inline asm in check_wakeup_from_wait()
>> won't be able to make a jump to the .L_wq_resume label defined in the
>> __prepare_to_wait() assembly block if the label is declared as
>> .L_wq_resume%=.
>>
>> Also we want to make sure there's a single .L_wq_resume seeing how
>> check_wakeup_from_wait() uses it as the restore entry point?
> Hmm, yes on both points; the %= would only work for .Lskip. Have you gained
> understanding why there is this duplication? The breaking out of the asm()
> that you do isn't going to be reliable, as in principle the compiler is
> still permitted to duplicate stuff. Afaict the only reliable way is to move
> the code to a separate assembly file (with the asm() merely JMPing there,
> providing a pseudo-return-address by some custom means). Or to a file-scope
> asm(), as those can't be duplicated.

See the simplified example in
https://github.com/llvm/llvm-project/issues/92161

When I debugged this a while back, The multiple uses of wqv->esp (one
explicit after the asm, one as an asm parameter) gain pointer
sanitisation, so the structure looks like:

    ...
    if ( bad pointer )
        __ubsan_report();
    asm volatile (...);
    if ( bad pointer )
        __ubsan_report();
    ...

which then got transformed to:

    if ( bad pointer )
    {
        __ubsan_report();
        asm volatile (...);
        __ubsan_report();
    }
    else
        asm volatile (...);

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.