From patchwork Thu Mar 14 03:35:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Fangrui Song X-Patchwork-Id: 12305 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 399F44493D9 for ; Thu, 14 Mar 2019 05:36:19 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 15A27689F00; Thu, 14 Mar 2019 05:36:19 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2ADB5689D7A for ; Thu, 14 Mar 2019 05:36:13 +0200 (EET) Received: by mail-qk1-f194.google.com with SMTP id z76so2504966qkb.12 for ; Wed, 13 Mar 2019 20:36:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=NFV8wS1ZGOiSgwCrp+VSQlgtEjSYwN1Il3/Cw4vFIOA=; b=JG6dmVRbY3vebwS62kwGemup/T3y2aJqyIRkQKqwY6QCL9/SHTUa13qOHpL954zgbr jTioudzbZmj8D1kQ17KELRGuvfweFllW+q8/w9yFVoYtypZqPOtkVRSTJKnZjbFVDxz+ fMFqCCJcmSUG876gK2AO8Brqi0+Jw1pSSyiztj3Epg365B974gHy9I8Iayt0S4o8Z0Ps Nk+sCfGA/DMt0JYXNe1Y7d13uW5cErrEQ7mwA6A0wMk6O/Zql/wG4Ovr1FfnvNiHEhUw 7weTt/P4fsqGqvEVI/W0etiRN8tXe+3JrPSlwuyge3Uv+Hqboho4Mqyn9PejFR76mqs+ 6V6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=NFV8wS1ZGOiSgwCrp+VSQlgtEjSYwN1Il3/Cw4vFIOA=; b=i0rj9u8rT695Fc8/36TdemtzRLiSwztWzw5+sn0Z/QBcyisjFBjQjYeGl91dI3cW/0 01YZWVJT0Idj/BqQQmxBwTVTsRYzP+jZpacGIpZUHQjEUxNjYJtNz8Q0Bf9ITko+h+xq lIG2d9XR9zOp84RobYmkK7kUvtlMeaScxZ8dorPqwJxYu7kQVClZ8Nad4sv39fYQGBly lTS16AmLLDIAjPFSzsoXGw+H+xNhhUg7MdVxBownnFfhQpBdckczXJC2bca8qhnnOlwc OGv0kF23uhT0YaiYPEJE+pUVrZom6zakx7DZP2+HKhKFs8SW+oMalN5TYpw2xQF12k6Q UqSQ== X-Gm-Message-State: APjAAAU3Fa3neXVZm/TgaYLYN2omjyI49i4xQv3oaXfANVDdjxjDwrR+ c6ronzJ6H8yM0mVoLix666VDJjxFV//lIItqMCClCKn2x1zJEA== X-Google-Smtp-Source: APXvYqz3klGDJ2qSKidovU7CfxQYLi5O/UZfLjfluTXYGZtxz6bbQlzskKtRBINrfXeHg4KAM7ngEpRe0mSVdfw6WJQ= X-Received: by 2002:ae9:c20f:: with SMTP id j15mr32513702qkg.132.1552534571220; Wed, 13 Mar 2019 20:36:11 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Date: Thu, 14 Mar 2019 11:35:59 +0800 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__ X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On Thu, Mar 14, 2019 at 10:36 AM Fāng-ruì Sòng wrote: > > On Thu, Mar 14, 2019 at 9:59 AM Henrik Gramner wrote: > > > > On Wed, Feb 20, 2019 at 8:03 PM Fāng-ruì Sòng > > wrote: > > > --- a/libavutil/mem.h > > > +++ b/libavutil/mem.h > > > > > > +#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__)) > > > + #define DECLARE_HIDDEN __attribute__ ((visibility ("hidden"))) > > > +#else > > > + #define DECLARE_HIDDEN > > > +#endif > > > > libavutil/mem.h is a public header so any defines added should have > > appropriate prefixes (yes, the existing defines violate this which is > > something that should be addressed, but that's a different issue). > > Do you have suggestions on the naming? av_declare_hidden? > > > Alternatively maybe those macros should be moved to some internal > > header because I don't really see any value of having inline asm > > support macros public. > > That would change too many files. I'd rather not do that in this patch. I still don't have access to my neomutt so I send the patch as an attached file via the webmail From 64fc0ce5fa80d6d2b6bc93f539cca48a844bc672 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Wed, 20 Feb 2019 16:25:46 +0800 Subject: [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__ Inline asm code assumes these DECLARE_ASM_ALIGNED declared global constants are non-preemptive, e.g. libavcodec/x86/cabac.h "lea "MANGLE(ff_h264_cabac_tables)", %0 \n\t" These constants are currently defined in C files with default visibility. Such object files cannot link to a shared object without -Wl,-Bsymbolic or -Wl,--version-script,libavcodec/libavcodec.ver: ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol ff_h264_cabac_tables; recompile with -fPIC It is better to express the intention explicitly and mark such global constants hidden. -Bsymbolic is dangerous as it breaks C++ semantics about address uniqueness of inline functions, type_info, ... --version-script can have the same problem unless used very carefully. DECLARE_ASM_CONST uses the "static" specifier to indicate internal linkage. The visibility annotation is unnecessary. Signed-off-by: Fangrui Song --- libavutil/mem.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libavutil/mem.h b/libavutil/mem.h index 5fb1a02dd9..98a14b826b 100644 --- a/libavutil/mem.h +++ b/libavutil/mem.h @@ -100,6 +100,12 @@ * @param v Name of the variable */ +#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__)) + #define av_visibility_hidden __attribute__ ((visibility ("hidden"))) +#else + #define av_visibility_hidden +#endif + #if defined(__INTEL_COMPILER) && __INTEL_COMPILER < 1110 || defined(__SUNPRO_C) #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (n))) v #define DECLARE_ASM_ALIGNED(n,t,v) t __attribute__ ((aligned (n))) v @@ -110,7 +116,7 @@ #define DECLARE_ASM_CONST(n,t,v) static const t av_used __attribute__ ((aligned (FFMIN(n, 16)))) v #elif defined(__GNUC__) || defined(__clang__) #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (n))) v - #define DECLARE_ASM_ALIGNED(n,t,v) t av_used __attribute__ ((aligned (n))) v + #define DECLARE_ASM_ALIGNED(n,t,v) t av_used __attribute__ ((aligned (n))) av_visibility_hidden v #define DECLARE_ASM_CONST(n,t,v) static const t av_used __attribute__ ((aligned (n))) v #elif defined(_MSC_VER) #define DECLARE_ALIGNED(n,t,v) __declspec(align(n)) t v -- 2.20.1