From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by sourceware.org (Postfix) with ESMTPS id 0635F385840C for ; Wed, 5 Jan 2022 19:47:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0635F385840C Received: by mail-pj1-x1031.google.com with SMTP id n30-20020a17090a5aa100b001b2b6509685so84353pji.3 for ; Wed, 05 Jan 2022 11:47:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bIQwDX8zHnwlJ1vQACxrtY4EykfkGkPNWiG4RL4DwXw=; b=Fc9J1f1eFOFxYj5FLzTMrupwTwxWVk8NvLpzTuvtr/8YJhBndAgelID9tEQRaVXo8U lqZAvdFycviiU7G/qCnNHgZ42fC9yOEa2rsSrnH0Cv2aU7Npenhspa0N9dex31zUE3AB JlatD0AIXmoAVTISQ0n8ZDaUm7Mgobeo89MRlg1RLj2BE1AvBEZhS90PDsh26etW+lAo vh/fdEUuh62RsbYh7GMCt1tybmW+PVDrcEoe2nEWuMAB6Fc8wcNa5ft9vdQToIYaa8US zizaFussS7z7WY/HCWqu/GGUZwmO9ZzirZlv+hbRZpfwmROUB13LWKgSgXWM4/oxMQ9v E1Jw== X-Gm-Message-State: AOAM530nDnR5MYTNsAtZlB/8KUclhiHo5QIC1MmQrKF3UrKHFDbbgg1n p4nDXdgy0GHPKtMnpXDDcbBPaVazOp/sdU4ji98= X-Google-Smtp-Source: ABdhPJwKK4EEhuhsFsJhs+RVq44u1Lt2on8+f/UVL+IpcnTQfFXj1djOQZ4NmG3dMyBJLaeCZ/mFjM9AfBQUo8HWPcI= X-Received: by 2002:a17:902:bf09:b0:149:d2a3:ddac with SMTP id bi9-20020a170902bf0900b00149d2a3ddacmr4443562plb.4.1641412057075; Wed, 05 Jan 2022 11:47:37 -0800 (PST) MIME-Version: 1.0 References: <20220105070726.3949144-1-vladimir.mezentsev@oracle.com> <3d619b7d-e0da-3c5a-b079-13d185a80fb1@suse.com> <3908cc3e-9df0-8413-b4c4-a125f783b670@oracle.com> In-Reply-To: <3908cc3e-9df0-8413-b4c4-a125f783b670@oracle.com> From: "H.J. Lu" Date: Wed, 5 Jan 2022 11:47:01 -0800 Message-ID: Subject: Re: [PATCH] opcodes/i386-dis.c is not thread-safe To: Vladimir Mezentsev Cc: Jan Beulich , Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3022.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Jan 2022 19:47:39 -0000 On Wed, Jan 5, 2022 at 11:19 AM Vladimir Mezentsev via Binutils wrote: > > > > On 1/5/22 03:42, Jan Beulich wrote: > > On 05.01.2022 08:07, Vladimir Mezentsev via Binutils wrote: > >> From: Vladimir Mezentsev > >> > >> We plan to use print_insn_i386_att, print_insn_i386_intel and > >> print_insn_i386 in a multithreaded application. > >> These functions are not thread safe due to the use of static variables. > >> > >> Tested on x86_64-pc-linux-gnu. > >> > >> opcodes/ChangeLog: > >> 2022-01-04 Vladimir Mezentsev > >> > >> * opcodes/i386-dis.c: Make print_insn_i386_att, print_insn_i386_intel > >> and print_insn_i386 thread-safe > > While I appreciate this step, I don't think the result is quite thread- > > safe yet. > > Why ? > > > > In particular the various abort() invocations aren't going to > > play well with a multi-threaded consumer of the library. And we know > > that there have been bugs in this area, i.e. where abort() would have > > got triggered by certain invalid encodings; > > It is a real bug if we see abort() in one thread application. > > But in a multi-threaded application, the problem is very easy to reproduce. > The problem is the shared variables are used in opcodes/i386-dis.c > without synchronization. > For example: > > % grep -n need_modrm i386-dis.c > 2404:static unsigned char need_modrm; <<<<<< the static shared variable > 2439:#define MODRM_CHECK if (!need_modrm) abort () <<<<<< Can be > abort() because need_modrm was reset for the other instruction. > 9682: need_modrm = twobyte_has_modrm[threebyte]; <<<<<< > Settings for two byte instruction > 9688: need_modrm = onebyte_has_modrm[*codep];<<<<<< Settings for > the other instruction > > > > -Vladimir > > > I'm pretty certain we haven't > > found (and eliminated) all of them just yet. > > > > Jan > > Hi, Vladimir, Please check it in. Thanks. -- H.J.