[
https://issues.apache.org/jira/browse/TS-2251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13777153#comment-13777153
]
Yunkai Zhang commented on TS-2251:
----------------------------------
But the code is simpler than before, it's ok for me.
> LogBuffer::destroy() defeated by compiler optimizations
> -------------------------------------------------------
>
> Key: TS-2251
> URL: https://issues.apache.org/jira/browse/TS-2251
> Project: Traffic Server
> Issue Type: Bug
> Components: Logging, Portability, Quality
> Reporter: James Peach
>
> {{LogBuffer::destroy()}} uses atomic compare and swaps on the {{LogBuffer}}
> reference count to decrement the refcount and destroy the LogBuffer object.
> However, the compiler (Apple clang-500.2.75) hoists the read of
> LogBuffer::m_references out of the loop, so it won't work correctly if
> {{ink_atomic_cas}} ever fails:
> {code}
> __ZN9LogBuffer7destroyEPS_: ## @_ZN9LogBuffer7destroyEPS_
> .cfi_startproc
> .cfi_personality 155, ___gxx_personality_v0
> Leh_func_begin1:
> .cfi_lsda 16, Lexception1
> Lfunc_begin1:
> .loc 1 66 0 ##
> /Users/jpeach/src/trafficserver.git/proxy/logging/LogBuffer.cc:66:0
> ## BB#0:
> pushq %rbp
> Ltmp13:
> .cfi_def_cfa_offset 16
> Ltmp14:
> .cfi_offset %rbp, -16
> movq %rsp, %rbp
> Ltmp15:
> .cfi_def_cfa_register %rbp
> pushq %r14
> pushq %rbx
> subq $32, %rsp
> Ltmp16:
> .cfi_offset %rbx, -32
> Ltmp17:
> .cfi_offset %r14, -24
> ##DEBUG_VALUE: destroy:lb <- RDI+0
> movq %rdi, %rbx
> {code}
> Notice that the following load of LogBuffer::m_references is outside the loop
> labelled {{LBB1_1}}:
> {code}
> Ltmp18:
> ##DEBUG_VALUE: destroy:lb <- RBX+0
> .loc 1 70 0 prologue_end ##
> /Users/jpeach/src/trafficserver.git/proxy/logging/LogBuffer.cc:70:0
> leaq 104(%rbx), %rsi
> Ltmp19:
> ##DEBUG_VALUE: ink_atomic_cas<int>:mem <- RSI+0
> .align 4, 0x90
> LBB1_1: ## =>This Inner Loop Header: Depth=1
> ##DEBUG_VALUE: destroy:lb <- RBX+0
> ##DEBUG_VALUE: ink_atomic_cas<int>:mem <- RSI+0
> movl (%rsi), %ecx
> Ltmp20:
> ##DEBUG_VALUE: old_ref <- ECX+0
> ##DEBUG_VALUE: ink_atomic_cas<int>:prev <- ECX+0
> .loc 1 71 0 ##
> /Users/jpeach/src/trafficserver.git/proxy/logging/LogBuffer.cc:71:0
> leal -1(%rcx), %edx
> Ltmp21:
> ##DEBUG_VALUE: ink_atomic_cas<int>:next <- EDX+0
> ##DEBUG_VALUE: new_ref <- EDX+0
> .loc 29 153 0 ##
> /Users/jpeach/src/trafficserver.git/lib/ts/ink_atomic.h:153:0
> movl %ecx, %eax
> lock
> cmpxchgl %edx, (%rsi)
> cmpl %ecx, %eax
> Ltmp22:
> .loc 1 73 0 ##
> /Users/jpeach/src/trafficserver.git/proxy/logging/LogBuffer.cc:73:0
> jne LBB1_1
> Ltmp23:
> ## BB#2:
> ##DEBUG_VALUE: destroy:lb <- RBX+0
> .loc 1 75 0 ##
> /Users/jpeach/src/trafficserver.git/proxy/logging/LogBuffer.cc:75:0
> testl %ecx, %ecx
> jle LBB1_15
> ## BB#3:
> ##DEBUG_VALUE: destroy:lb <- RBX+0
> .loc 1 77 0 ##
> /Users/jpeach/src/trafficserver.git/proxy/logging/LogBuffer.cc:77:0
> testl %edx, %edx
> jne LBB1_14
> ## BB#4:
> ##DEBUG_VALUE: destroy:lb <- RBX+0
> testq %rbx, %rbx
> jne LBB1_5
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
