public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH 3/5] [gdb/symtab] Work around fsanitize=address false positive for per_cu->lang
Date: Wed, 29 Jun 2022 17:29:12 +0200	[thread overview]
Message-ID: <20220629152914.13149-3-tdevries@suse.de> (raw)
In-Reply-To: <20220629152914.13149-1-tdevries@suse.de>

When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a data race between:
...
  Read of size 1 at 0x7b200000300d by thread T2:^M
    #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
    abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6164 \
    (gdb+0x82ec95)^M
...
and:
...
  Previous write of size 1 at 0x7b200000300d by main thread:^M
    #0 prepare_one_comp_unit gdb/dwarf2/read.c:23588 (gdb+0x86f973)^M
...

In other words, between:
...
  if (this_cu->reading_dwo_directly)
...
and:
...
    cu->per_cu->lang = pretend_language;
...

Both fields are part of the same bitfield, and writing to one field while
reading from another is not a problem, so this is a false positive.

An easy way to get rid of the false positive when compiling with thread
sanitizer is to do this:
...
  #ifdef __SANITIZE_THREAD__
    language lang;
  #else
    ENUM_BITFIELD (language) lang : LANGUAGE_BITS;
  #endif
...
but that also inhibits the detection of parallel writing to different fields
in the same bitfield, which is a problem.

Fix this instead by moving the lang field out of the bitfield.

In the bitfield, storing the lang field required LANGUAGE_BITS == 5 bits.

Set the underlying type of enum lang to char, to require only 8 bits outside
the bitfield.

Due to a compilation error with gcc 7.5.0, that also requires us to set
LANGUAGE_BITS to 8.

The size of struct dwarf2_per_cu_data remains the same (at least for -m64).

Tested on x86_64-linux.
---
 gdb/defs.h        |  4 ++--
 gdb/dwarf2/read.h | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index 99bfdd526ff..d5bf7cb2778 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -207,7 +207,7 @@ extern void quit_serial_event_clear (void);
    ada_sniff_from_mangled_name).  (Keep this order in sync with the
    'languages' array in language.c.)  */
 
-enum language
+enum language : char
   {
     language_unknown,		/* Language not known */
     language_auto,		/* Placeholder for automatic setting */
@@ -229,7 +229,7 @@ enum language
 
 /* The number of bits needed to represent all languages, with enough
    padding to allow for reasonable growth.  */
-#define LANGUAGE_BITS 5
+#define LANGUAGE_BITS 8
 gdb_static_assert (nr_languages <= (1 << LANGUAGE_BITS));
 
 enum precision_type
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 51e02dfc457..db300b19621 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -99,7 +99,8 @@ typedef std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>
 struct dwarf2_per_cu_data
 {
   dwarf2_per_cu_data ()
-    : queued (false),
+    : lang (language_unknown),
+      queued (false),
       is_debug_types (false),
       is_dwz (false),
       reading_dwo_directly (false),
@@ -109,7 +110,6 @@ struct dwarf2_per_cu_data
       mark (false),
       files_read (false),
       unit_type {},
-      lang (language_unknown),
       scanned (false)
   {
   }
@@ -125,6 +125,9 @@ struct dwarf2_per_cu_data
   /* DWARF standard version this data has been read from (such as 4 or 5).  */
   unsigned char dwarf_version = 0;
 
+  /* The language of this CU.  */
+  language lang;
+
   /* Flag indicating this compilation unit will be read in before
      any of the current compilation units are processed.  */
   unsigned int queued : 1;
@@ -174,9 +177,6 @@ struct dwarf2_per_cu_data
   /* The unit type of this CU.  */
   ENUM_BITFIELD (dwarf_unit_type) unit_type : 8;
 
-  /* The language of this CU.  */
-  ENUM_BITFIELD (language) lang : LANGUAGE_BITS;
-
   /* True if this CU has been scanned by the indexer; false if
      not.  */
   std::atomic<bool> scanned;
-- 
2.35.3


  parent reply	other threads:[~2022-06-29 15:29 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-29 15:29 [PATCH 1/5] [COVER-LETTER, RFC] Fix some fsanitize=thread issues in gdb's cooked index Tom de Vries
2022-06-29 15:29 ` [PATCH 2/5] [gdb/symtab] Fix data race on per_cu->dwarf_version Tom de Vries
2022-07-01 11:16   ` Tom de Vries
2022-07-02 11:07     ` Tom de Vries
2022-07-04 18:51       ` Tom Tromey
2022-07-04 19:43         ` Tom de Vries
2022-07-04 19:53           ` Tom Tromey
2022-06-29 15:29 ` Tom de Vries [this message]
2022-06-29 17:38   ` [PATCH 3/5] [gdb/symtab] Work around fsanitize=address false positive for per_cu->lang Pedro Alves
2022-06-29 18:25     ` Pedro Alves
2022-06-29 18:28       ` Pedro Alves
2022-07-04  7:04         ` [PATCH 3/5] [gdb/symtab] Work around fsanitize=address false positive for per_ cu->lang Tom de Vries
2022-07-04 18:32   ` [PATCH 3/5] [gdb/symtab] Work around fsanitize=address false positive for per_cu->lang Tom Tromey
2022-07-04 19:45     ` Tom de Vries
2022-07-06 19:20       ` [PATCH] Introduce struct packed template, fix -fsanitize=thread for per_cu fields Pedro Alves
2022-07-07 10:18         ` Tom de Vries
2022-07-07 15:26           ` Pedro Alves
2022-07-08 14:54             ` Tom de Vries
2022-07-12 10:22               ` Tom de Vries
2022-06-29 15:29 ` [PATCH 4/5] [gdb/symtab] Work around fsanitize=address false positive for per_cu->unit_type Tom de Vries
2022-06-29 15:29 ` [PATCH 5/5] [gdb/symtab] Fix data race on per_cu->lang Tom de Vries
2022-07-04 18:30   ` Tom Tromey
2022-07-05  8:17     ` Tom de Vries
2022-07-05 15:19     ` Tom de Vries
2022-07-06 15:42       ` Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220629152914.13149-3-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).