Uploaded image for project: 'Traffic Server'
  1. Traffic Server
  2. TS-2251

LogBuffer::destroy() defeated by compiler optimizations

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.2.0
    • Component/s: Logging, Portability, Tests
    • Labels:
      None

      Description

      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:

      __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
      

      Notice that the following load of LogBuffer::m_references is outside the loop labelled LBB1_1:

      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
      

        Attachments

          Activity

            People

            • Assignee:
              jamespeach James Peach
              Reporter:
              jamespeach James Peach
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: