From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 1278C3858D3C for ; Mon, 25 Mar 2024 16:00:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1278C3858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1278C3858D3C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711382435; cv=none; b=HbkfNNnHYTrZM9rs0QL4U9S6n6x3+zGElhJ94zmKritm/RIYLYvTkvP+1vyde0ghW+JOLJFKOEhMo8/+UJFTCm7t+/lpZd2Q3mOphIOFda9zxxpzmXo9QvjJFK/hRrW3KMqCGERf8R75TBiqERLmpPCAcZQdPZBslAtlWQgnHiI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711382435; c=relaxed/simple; bh=hH9Z1wLSnwcemBhd5blaJHXc3pc3OtD9wEdSlFfD4BM=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=H1f4ZBsKsw+0xfBz4zuVS3Fn9oL4GqXQiGeAf0JkLSvlzpPQy+iTPK/QjG3kA8Bgf9qf8JcYWWCslaDGiADKOHZyG2B/qfdFGDx6Bq6RCtEfCTmi5D1UNU8k2yqzViTH7IsWL6YSSVbuWq8EAwfu0WAnZjOEZJIaR3HLAs/6Jkc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711382433; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=FYYywJwevEQfTw/pjtiJ9C3kPmf/D9UB588uClHthzM=; b=howZjLW/0H3hz9BW6CbGFjkgUPjSaU/wfAWApOcl0K2nZcsL0VqhfWzTQoWEzvuxM2TLGm LRd75veIWG8ynHAe8fx/GrAgYlvV05s2DhI3RHkv243b/mDVJYGDlZxH3i18zm3ELoXpIi bts9ymhPuTyFSyDq+t64V7K7j8bhOiM= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-640-KqP5i3ktMV2DnGUf4Lj6TQ-1; Mon, 25 Mar 2024 12:00:32 -0400 X-MC-Unique: KqP5i3ktMV2DnGUf4Lj6TQ-1 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-7817253831cso637818885a.0 for ; Mon, 25 Mar 2024 09:00:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711382430; x=1711987230; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=FYYywJwevEQfTw/pjtiJ9C3kPmf/D9UB588uClHthzM=; b=DFzI+pa0z4DlwDhovhkfhQbiLumTMTlxTQ90t1pmXSp6JLS/Ncek+9LxVp5UvMLsJp +iMg8LenOyE4QfF6pb32pnn+/tPrvffbDBTSPRbutZSujBRTi3rcBsCQlB98NZOjScIs noyI/DqqPTxO8sBjOn0J8yy/hWQtSSuwK890dZbN6jyG3XykPmOtrGrUeAyU5XU5dLOX TCKqinbPUnIuTsAwtb0plgOmJG/1GczFx/yIzHOjUQ+QPp0yCHJYTyslj1ztSN89xxqf fExQaD8W5v0fG+a2ktMIJFXD/8AJDUxcC8aE0ugLbdD8YMoA9JammegrDiw/FMRtnlpD ceGw== X-Gm-Message-State: AOJu0YyNo8EgHS0ZLxWrVCDUCpa0nRCfWq4+e1jG9qrSUs3rUvc6w6DZ R0d1Pdxqi2t1AsbiK6r0Z1BxRraA84ggw9ZECkhz/WC49Z6z4q6AwBWvNDMvCpg0oFPu2Y57OZ/ Z9ov6Jik3jDjwC/UPc4wXgQaDm0eYuAr9U/nwgsU6gAhXt+2o6Uima3P++NPe38hM8ymLEGZSxW 0CzuUrdx5E/+9M1ICnl8UPq2p7eBMqT7O1bA== X-Received: by 2002:a05:620a:5e4a:b0:78a:51f7:e90d with SMTP id ya10-20020a05620a5e4a00b0078a51f7e90dmr4912915qkn.56.1711382430743; Mon, 25 Mar 2024 09:00:30 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFApGhqqzhcbkErGxqKii1eM0LAxvU1Odv47fsffZfOVgQ6PvvRp5GsAmtd01lERcEvR3dN0w== X-Received: by 2002:a05:620a:5e4a:b0:78a:51f7:e90d with SMTP id ya10-20020a05620a5e4a00b0078a51f7e90dmr4912895qkn.56.1711382430381; Mon, 25 Mar 2024 09:00:30 -0700 (PDT) Received: from [192.168.1.18] ([79.123.79.31]) by smtp.gmail.com with ESMTPSA id u4-20020a05620a022400b00789fa326156sm2224321qkm.82.2024.03.25.09.00.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 25 Mar 2024 09:00:30 -0700 (PDT) Message-ID: Date: Mon, 25 Mar 2024 16:00:28 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Add support for global-file-syms to the clang and llvm plugins To: annobin@sourceware.org, Tulio Magno Quites Machado Filho Cc: grant@hcubed.com References: <20240322201736.421134-1-tuliom@ascii.art.br> From: Nick Clifton Autocrypt: addr=nickc@redhat.com; keydata= xsFNBFm/2cUBEADkvRqMWfAryJ52T4J/640Av5cam9ojdFih9MjcX7QWFxIzJfTFYq2z+nb4 omdfZosdCJL2zGcn6C0AxpHNvxR9HMDkEyFHKrjDh4xWU+pH4z9azQEqJh331X7UzbZldqQo 16VkuVavgsTJaHcXm+nGIBTcUbl2oiTtHhmuaYxx6JTMcFjC7vyO5mLBw78wt52HBYweJ0Nj HBvvH/JxbAAULSPRUC61K0exlO49VFbFETQNG1hZTKEji95fPbre7PpXQ0ewQShUgttEE/J3 UA4jYaF9lOcZgUzbA27xTV//KomP0D30yr4e4EJEJYYNKa3hofTEHDXeeNgM25tprhBUMdbV RZpf2Keuk2uDVwc+EiOVri48rb1NU+60sOXvoGO6Ks81+mhAGmrBrlgLhAp8K1HPHI4MG4gH nrMqX2rEGUGRPFjC3qqVVlPm8H05PnosNqDLQ1Pf7C0pVgsCx6hKQB7Y1qBui7aoj9zeFaQg pYef+CEERIKEcWwrjaOJwK3pi9HFdxS0NNWYZj8HPzz/AsgTTQdsbulPlVq2SsctmOnL42CZ OCTppGYwl53CG/EqVY+UQBzFzJBaY8TJRFFYVEy5/HH4H11rMoZwqIkk71EOGU3X6mWlANRi kR3M4GhVITRzuaV69Fed+OeXcCmP94ASLfuhBR2uynmcHpBKpwARAQABzTtOaWNrIENsaWZ0 b24gKENoaWVmIEJpbnV0aWxzIE1haW50YWluZXIpIDxuaWNrY0ByZWRoYXQuY29tPsLBeAQT AQIAIgUCWb/ZxQIbAwYLCQgHAwIGFQgCCQoLBBYCAwECHgECF4AACgkQE/zvid2ePE9cOxAA 3cX1bdDaTFttTqukdPXLCtD2aNwJos4vB4LYPSgugLkYaHIQH9d1NQPhS0TlUeovnFNESLaV soihv0YmBUCyL4jE52FRoTjE6fUhYkFNqIWN2HYwkVrSap2UUJFquRVoVbPkbSup8P+D8eyd BbdxsY6f+5E8Rtz5ibVnPZTib7CyqnFokJITWjzGdIP0Gn+JWVa6jtHTImWx1MtqiuVRDapU hrIoUIjf98HQn9/N5ylEFYQTw7tzaJNWeGUoGYS8+8n/0sNbuYQUU/zwMVY9wpJcrXaas6yZ XGpF/tua59t9LFCct+07YAUSWyaBXqBW3PKQz7QP+oE8yje91XrhOQam04eJhPIBLO88g6/U rdKaY7evBB8bJ76Zpn1yqsYOXwAxifD0gDcRTQcB2s5MYXYmizn2GoUm1MnCJeAfQCi/YMob R+c8xEEkRU83Tnnw3pmAbRU6OcPihEFuK/+SOMKIuV1QWmjkbAr4g9XeXvaN+TRJ9Hl/k1k/ sj+uOfyGIaFzM/fpaLmFk8vHeej4i2/C6cL4mnahwYBDHAfHO65ZUIBAssdA6AeJ+PGsYeYh qs6zkpaA2b0wT4f9s7BPSqi0Veky8bUYYY7WpjzDcHnj1gEeIU55EhOQ42dnEfv7WrIAXanO P8SjhgqAUkb3R88azZCpEMTHiCE4bFxzOmjOwU0EWb/ZxQEQALaJE/3u23rTvPLkitaTJFqK kwPVylzkwmKdvd2qeEFk1qys2J3tACTMyYVnYTSXy5EJH2zJyhUfLnhLp8jJZF4oU5QehOaJ PcMmzI/CZS1AmH+jnm6pukdZAowTzJyt4IKSapr+7mxcxX1YQ2XewMnFYpLkAA2dHaChLSU/ EHJXe3+O4DgEURTFMa3SRN/J4GNMBacKXnMSSYylI5DcIOZ/v0IGa5MAXHrP1Hwm1rBmloIc gmzexczBf+IcWgCLThyFPffv+2pfLK1XaS82OzBC7fS01pB/eDOkjQuKy16sKZX6Rt57vud4 0uE5a0lpyItC2P7u7QWL4yT5pMF+oS8bm3YWgEntV380RyZpqgJGZTZLNq2T4ZgfiaueEV4J zOnG2/QRGjOUrNQaYzKy5V127CTnRg4BYF/uLEmizLcI3O3U1+mEz6h48wkAojO1B6AZ8Lm+ JuxOW5ouGcrkTEuIG56GcDwMWS/Pw/vNsDyNmOCjy9eEKWJgmMmLaq59HpfTd8IOeaYyuAQH AsYt/zzKy0giMgjhCQtuc99E4nQE9KZ44DKsnqRabK9s3zYE3PIkCFIEZcUiJXSXWWOIdJ43 j+YyFHU5hqXfECM6rzKGBeBUGTzyWcOX6YwRM4LzQDVJwYG8cVfth+v4/ImcXR43D4WVxxBE AjKag02b+1yfABEBAAHCwV8EGAECAAkFAlm/2cUCGwwACgkQE/zvid2ePE/dqQ/6ApUwgsZz tps0MOdRddjPwz44pWXS5MG45irMQXELGQyxkrafc8lwHeABYstoK8dpopTcJGE3dZGL3JNz 1YWxQ5AV4uyqBn5N8RubcA8NzR6DQP+OGPIwzMketvVC/cbbKDZqf0uTDy3jP65OFhSkTEIy nYv1Mb4JJl3Sq+haUbfWLAV5nboSuHmiZE6Bz2+TjdoVkNwHBfpqxu6MlWka+P98SUcmY8iV hPy9QC1XFOGdFDFf1kYgHW27mFwds35NQhNARgftAVz9FZXruW6tFIIfisjr3rVjD9R8VgL7 l5vMr9ylOFpepnI6+wd2X1566HW7F1Zw1DIrY2NHL7kL5635bHrJY4n7o/n7Elk/Ca/MAqzd IZxz6orfXeImsqZ6ODn4Y47PToS3Tr3bMNN9N6tmOPQZkJGHDBExbhAi/Jp8fpWxMmpVCUl6 c85cOBCR4s8tZsvGYOjR3CvqKrX4bb8GElrhOvAJa6DdmZXc7AyoVMaTvhpq3gJYKmC64oqt 7zwIHwaCxTbP6C6oUp9ENRV7nHnXN3BlvIgCo4QEs6HkDzkmgYlCEOKBiDyVMSkPDZdsspa+ K4GlU2Swi/BDJMjtDxyo+K0M81LXXxOeRfEIfPtZ3ddxBKPva1uSsuz+pbN9d1JY8Ko5T/h1 6susi2ReUyNJEJaSnjO5z13TQ1U= In-Reply-To: <20240322201736.421134-1-tuliom@ascii.art.br> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-GB Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Tulio, > This part starts by moving the implementation of parse_env() to > annobin-common.cc in order to be shared between the gcc and the llvm > plugins. > > This was required because the llvm plugin didn't have a mechanism to > receive arguments from the user. This commit adds support for using the > environment variable ANNOBIN for the LLVM plugin, although it's still > missing many of the basic features available in the other plugins. > > Both clang and llvm plugins are now able to generate the same output as > the GCC plugin, e.g. _annobin_hello_c_1711138217_00204218_start. Thanks very much for writing this patch. I do have a few comments on the code, but overall I thought that it was excellent. > diff --git a/annobin-common.cc b/annobin-common.cc > +extern bool parse_argument (const char *, const char *); There is no external function called parse_argument() so you do not have to provide a prototype for it... > +void > +parse_env (bool (* parse_argument)(const char *, const char *)) I think that the function should be called 'annobin_parse_env' rather than 'parse_env'. There are two reasons: 1) there is a distinct possibility that it might clash with a same-named function in the sources of Clang, LLVM or GCC one day. 2) when running the compiler inside a debugger it is helpful to have function names that indicate to which part of the program they belong. Thus a user using gdb to debug a clang crash for example might run across "parse_env" and have no idea where to look for it in the sources. But if they see "annobin_parse_env" they immediately have a big clue for where to look. Also - the function should return a BOOL based upon the results of calling parse_argument(). Ie if any invocation of parse_argument() fails, then parse_env() should also fail. Also - the parse_argument function should take a third argument, a void * pointer provided to parse_env() by the caller. The reason for this is that the caller may wish to pass extra information to the parse_argument function. > +{ > + char arg[2048]; Large buffers on the stack are a security hazzard. Better to make the buffer static - or better yet, dynamic. > + comma = strchr (env, ','); > + if (comma) > + { > + strncpy (arg, env, comma - env); Potential buffer overflow if (common - env >= sizeof arg) > diff --git a/annobin-common.h b/annobin-common.h > +void parse_env (bool (* parse_argument)(const char *, const char *)); > +#endif // ANNOBIN-COMMON_H_ I am a firm believer in header files providing descriptions of the functions that they prototype. This makes it easier for users unfamiliar with the source code to use the header without having to delve into the sources themselves. So in the version of your patch that I have checked in I added: /* Checks for an ANNOBIN environment variable. If it exists, parses it for comma separated command line options, passing each in turn to PARSE_ARGUMENT. Arguments are copied into a buffer before processing and can be manipulated by PARSE_ARGUMENT. If an argument contains a '=' character then it is split into two pieces and passed to PARSE_ARGUMENT as (NAME, VALUE). Otherwise it is just passed as (NAME, ""). The return value from PARSE_ARGUMENT is recorded, but parsing will continue even if PARSE_ARGUMENT returns FALSE. Returns TRUE if the ANNOBIN environment variable does not exist. Returns TRUE if the ANNOBIN environment exists and PARSE_ARGUMENT returns TRUE for all of the arguments. Return FALSE otherwise. */ I also thought that we should be consistent and allow the clang plugin to also parse the ANNOBIN environment variable. So I added that. But basically I was very happy with your patch and I have now checked it in to the annobin repository. Cheers Nick PS. I have not built the rawhide version yet as I have run into some problems with the testsuites...