From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by sourceware.org (Postfix) with ESMTPS id 5B1F538930E0 for ; Mon, 8 Feb 2021 12:20:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5B1F538930E0 Received: by mail-lf1-x12c.google.com with SMTP id a12so21941746lfb.1 for ; Mon, 08 Feb 2021 04:20:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=9n+HajoBVAxp3VnE/lsuxfd4Xz29gh0+HIdWvSWRido=; b=IBbwz1XHKzcIvVheVDmP5uHr5+Y5QoOoK1Ik6n2T0y3HopcX81gCL520fdXOcp4pa3 RmDPYPdmbAdWjgFe3FpbztEcna1ej99eHHW7HErs44g+CZAtNp6FmnubkeXFLMIGA7Q2 M+dDeLsy9yFatJ92Cu8JBDXGd7lT7ahQmf//2Ot3g3uoMjPEWkPHg4q6HIlRGBgl4a0b Ci+5yHSJzJlhM6nObimqVdHyjr9NA9tmF9fuxQoWaXH2wsTfCC9VsX0+ELok+e1XOyHT CO6NxfUwgtXF8UrD30wJPXUJ7Z1FdMDG8Q6gA59PHA/ZX8IZkV4aBaqqGBKaiwhiPOhE Vy0g== X-Gm-Message-State: AOAM532vQbHng54JWEZ5QIpPjmafCatW8gnpFhErMM81kTFOndktq3uQ OaEp/11Q+SSv4WhRUUZWnHT/k8juYZgfLiWu X-Google-Smtp-Source: ABdhPJzh8C0LLC5rgD+4yfYKprsbTvRETmkRRT8eoHXHjo4IEli5GYRrFqZ+PRIdI5QhozIPaO5JDw== X-Received: by 2002:a19:24d5:: with SMTP id k204mr2145433lfk.126.1612786805101; Mon, 08 Feb 2021 04:20:05 -0800 (PST) Received: from atlantis.home ([2a03:1b20:4:f011::14d]) by smtp.gmail.com with ESMTPSA id p8sm785345ljn.83.2021.02.08.04.20.03 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Feb 2021 04:20:04 -0800 (PST) From: Shahab Vahedi To: gdb-patches@sourceware.org Cc: Shahab Vahedi , Shahab Vahedi , Simon Marchi , Tom Tromey , Luis Machado , Francois Bedard Subject: [PUSHED gdb-10-branch] gdb: Do not interrupt atomic sequences for ARC Date: Mon, 8 Feb 2021 13:20:31 +0100 Message-Id: <20210208122031.18484-1-shahab.vahedi@gmail.com> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210125230230.12250-1-shahab.vahedi@gmail.com> References: <20210125230230.12250-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Feb 2021 12:20:08 -0000 From: Shahab Vahedi When stepping over thread-lock related codes (in uClibc), the inferior process gets stuck and never manages to enter the critical section: ------8<------- 1 size_t fwrite(const void * __restrict ptr, size_t size, 2 size_t nmemb, register FILE * __restrict stream) 3 { 4 size_t retval; 5 __STDIO_AUTO_THREADLOCK_VAR; 6 7 > __STDIO_AUTO_THREADLOCK(stream); 8 9 retval = fwrite_unlocked(ptr, size, nmemb, stream); 10 11 __STDIO_AUTO_THREADUNLOCK(stream); 12 13 return retval; 14 } ------>8------- Here, we are at line 7. Using the "next" command leads no where. However, setting a breakpoint on line 9 and issuing "continue" works. Looking at the assembly instructions reveals that we're dealing with the critical section entry code [1] that should never be interrupted, in this case by the debugger's implicit breakpoints: ------8<------- ... 1 add_s r0,r13,0x38 2 mov_s r3,1 3 llock r2,[r0] <-. 4 brne.nt r2,0,14 --. | 5 scond r3,[r0] | | 6 bne -10 --|--' 7 brne_s r2,0,84 <-' ... ------>8------- Lines 3 until 5 (inclusive) are supposed to be executed atomically. Therefore, GDB should never (implicitly) insert a breakpoint on lines 4 and 5, else the program will try to acquire the lock again by jumping back to line 3 and gets stuck in an infinite loop. The solution is to make GDB aware of these patterns so it inserts breakpoints after the sequence -- line 6 in this example. [1] https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/arc/bits/atomic.h#n46 ------8<------- ({ \ __typeof(oldval) prev; \ \ __asm__ __volatile__( \ "1: llock %0, [%1] \n" \ " brne %0, %2, 2f \n" \ " scond %3, [%1] \n" \ " bnz 1b \n" \ "2: \n" \ : "=&r"(prev) \ : "r"(mem), "ir"(oldval), \ "r"(newval) /* can't be "ir". scond can't take limm for "b" */\ : "cc", "memory"); \ \ prev; \ }) ------>8------- "llock" (Load Locked) loads the 32-bit word pointed by the source operand. If the load is completed without any interruption or exception, the physical address is remembered, in Lock Physical Address (LPA), and the Lock Flag (LF) is set to 1. LF is a non-architecturally visible flag and is cleared whenever an interrupt or exception takes place. LF is also cleared (atomically) whenever another process writes to the LPA. "scond" (Store Conditional) will write to the destination address if and only if the LF is set to 1. When finished, with or without a write, it atomically copies the LF value to ZF (Zero Flag). These two instructions together provide the mechanism for entering a critical section. The code snippet above comes from uClibc: ----------------------- v3 (after Tom's remarks[2]): handle_atomic_sequence() - no need to initialize the std::vector with "{}" - fix typo in comments: "conditial" -> "conditional" - add braces to the body of "if" condition because of the comment line arc_linux_software_single_step() - make the performance slightly more efficient by moving a few variables after the likely "return" point. v2 (after Simon's remarks[3]): - handle_atomic_sequence() gets a copy of an instruction instead of a reference. - handle_atomic_sequence() asserts if the given instruction is an llock. [2] https://sourceware.org/pipermail/gdb-patches/2021-February/175805.html [3] https://sourceware.org/pipermail/gdb-patches/2021-January/175487.html gdb/ChangeLog: PR tdep/27369 * arc-linux-tdep.c (handle_atomic_sequence): New. (arc_linux_software_single_step): Call handle_atomic_sequence(). --- gdb/ChangeLog | 6 ++++ gdb/arc-linux-tdep.c | 77 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index fde6f9c3830..9d12c466952 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2021-02-08 Shahab Vahedi + + PR tdep/27369 + * arc-linux-tdep.c (handle_atomic_sequence): New. + (arc_linux_software_single_step): Call handle_atomic_sequence(). + 2021-02-05 Tom de Vries PR breakpoints/27330 diff --git a/gdb/arc-linux-tdep.c b/gdb/arc-linux-tdep.c index 1fef62a6128..a7bace12623 100644 --- a/gdb/arc-linux-tdep.c +++ b/gdb/arc-linux-tdep.c @@ -155,6 +155,78 @@ arc_linux_sw_breakpoint_from_kind (struct gdbarch *gdbarch, : arc_linux_trap_s_le); } +/* Check for an atomic sequence of instructions beginning with an + LLOCK instruction and ending with a SCOND instruction. + + These patterns are hand coded in libc's (glibc and uclibc). Take + a look at [1] for instance: + + main+14: llock r2,[r0] + main+18: brne.nt r2,0,main+30 + main+22: scond r3,[r0] + main+26: bne main+14 + main+30: mov_s r0,0 + + If such a sequence is found, attempt to step over it. + A breakpoint is placed at the end of the sequence. + + This function expects the INSN to be a "llock(d)" instruction. + + [1] + https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/ \ + sysdeps/linux/arc/bits/atomic.h#n46 + */ + +static std::vector +handle_atomic_sequence (arc_instruction insn, disassemble_info &di) +{ + const int atomic_seq_len = 24; /* Instruction sequence length. */ + std::vector next_pcs; + + /* Sanity check. */ + gdb_assert (insn.insn_class == LLOCK); + + /* Data size we are dealing with: LLOCK vs. LLOCKD */ + arc_ldst_data_size llock_data_size_mode = insn.data_size_mode; + /* Indicator if any conditional branch is found in the sequence. */ + bool found_bc = false; + /* Becomes true if "LLOCK(D) .. SCOND(D)" sequence is found. */ + bool is_pattern_valid = false; + + for (int insn_count = 0; insn_count < atomic_seq_len; ++insn_count) + { + arc_insn_decode (arc_insn_get_linear_next_pc (insn), + &di, arc_delayed_print_insn, &insn); + + if (insn.insn_class == BRCC) + { + /* If more than one conditional branch is found, this is not + the pattern we are interested in. */ + if (found_bc) + break; + found_bc = true; + continue; + } + + /* This is almost a happy ending. */ + if (insn.insn_class == SCOND) + { + /* SCOND should match the LLOCK's data size. */ + if (insn.data_size_mode == llock_data_size_mode) + is_pattern_valid = true; + break; + } + } + + if (is_pattern_valid) + { + /* Get next instruction after scond(d). There is no limm. */ + next_pcs.push_back (insn.address + insn.length); + } + + return next_pcs; +} + /* Implement the "software_single_step" gdbarch method. */ static std::vector @@ -168,8 +240,11 @@ arc_linux_software_single_step (struct regcache *regcache) struct arc_instruction curr_insn; arc_insn_decode (regcache_read_pc (regcache), &di, arc_delayed_print_insn, &curr_insn); - CORE_ADDR next_pc = arc_insn_get_linear_next_pc (curr_insn); + if (curr_insn.insn_class == LLOCK) + return handle_atomic_sequence (curr_insn, di); + + CORE_ADDR next_pc = arc_insn_get_linear_next_pc (curr_insn); std::vector next_pcs; /* For instructions with delay slots, the fall thru is not the -- 2.30.0