Received-SPF: pass (google.com: domain of linux-kernel+bounces-176090-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <b28e6bcb-dde2-4ac0-ac0d-dfddb42c4426@nvidia.com>
Date: Fri, 10 May 2024 11:22:16 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] selftests/openat2: fix clang build failures:
 -static-libasan, LOCAL_HDRS
From: John Hubbard <jhubbard@nvidia.com>
To: Ryan Roberts <ryan.roberts@arm.com>, Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
 =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
 Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>,
 Christian Brauner <brauner@kernel.org>,
 Muhammad Usama Anjum <usama.anjum@collabora.com>,
 Alexey Gladkov <legion@kernel.org>, Valentin Obst <kernel@valentinobst.de>,
 linux-kselftest@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
 llvm@lists.linux.dev
References: <20240504044336.14411-1-jhubbard@nvidia.com>
 <c406383d-e08a-4a12-9e25-1c987b0d678f@arm.com>
 <f715719a-c835-496c-9e99-d249e5607a0b@nvidia.com>
Content-Language: en-US
In-Reply-To: <f715719a-c835-496c-9e99-d249e5607a0b@nvidia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?VDFzeUE5NzlIb0JpRnBteVhpd3dzWkRYcllGaVdla0Z0cW1GV285dnVveE1k?=
 =?utf-8?B?aklNVGZJOTRWTW4xSng0SGEreDFoRitmUjcvZURxNHVPNlV5YUc2U0dFV1FO?=
 =?utf-8?B?VDN2bEZSSEVYa1BsMmhxZ3NlY2RsR3pFcmFKRFFQaXpiTlNiREMybVlKVXVV?=
 =?utf-8?B?Y25tRmVXOUNaajUzZ1dqYlY0bXVjVnRPOWIzbHI3RG5GYU92SWF2SkFjQTBM?=
 =?utf-8?B?enVRMTZFRWdPWnVDdDV6SnowdFVLNEZCU0Q4QlltNWI0T0ZGYXplUUhNWGtX?=
 =?utf-8?B?R0paTFA4dE5BOG95b1oxbzVLS1hlL2NVVFJmZjhNUDdBSHA0U0ZIOWl3djAr?=
 =?utf-8?B?UzFjc0lDV1VKOXROOGZaL1BBR0tST3ZUc051eGY4MnRMa3AyaHRPWGhtblhT?=
 =?utf-8?B?RWw5NlFoYkxEeWJEdDN5M290T2gzNEUxWjBZWnQ2clpnSzJDVlNCeXRiWWpk?=
 =?utf-8?B?OVV6ekpRUkM4YlFLdHBWUVhBZjZOUEN6aUNTS3FSQzhCdHRBaFk1ckJ5alh3?=
 =?utf-8?B?MDhISGdBSTNoYWJQZWlTTjJtMDVRMUk1ZXAxL1duYllQdzNJWmREZDB3Q0hn?=
 =?utf-8?B?bElIL1pRcmdNeFFRbG42cUZveDlQQTdKUkFyMHI2ZTdUQ0NOdzRTNFZGblVO?=
 =?utf-8?B?NkI2VERZdzF6Q3Jkb29MQWUzUEFjaEM4bnJmMnRsVG9GVml5U3NzSDhBVjFt?=
 =?utf-8?B?OUtFdW9DV0I2cy9jTENkcUU1dHBWTnF6dDEvTnVlRXZWNm01UDVuYUhpdXRr?=
 =?utf-8?B?Y2ZlNktOWk9MNDFWUkJtaWZWdys3WGcya1FacUcyemhmcnJTTW1YVHFnZ3Ir?=
 =?utf-8?B?RUtMQkJ4NytjMVRBWitQVFltbUh5SkNkcFJsdEo5eFh5QTNJRGNvRndRVmtE?=
 =?utf-8?B?M0t3dm9EMmdmTWxHMSt2RlFPQmxqM1lwRjBobW04QzlzZWJ2RmJHcm9DK1J0?=
 =?utf-8?B?TFB0YlE0aXpRVnZKNXFjd0JoRitSdkQrWlU2Z2F0WVloWWVleGFVZzc1YzV3?=
 =?utf-8?B?N3VHdVJSdm1CUWtvNk05V201bjVPTXZhWEhzanhPdnlxUWtmY1BIcUMvRUVK?=
 =?utf-8?B?YkZ1R0pSZDB6dXBjSnRNYWdoc0s3TjJOY2hXaktCUllRZnlTbzF0N1NCR09h?=
 =?utf-8?B?aFlKSHZCeGd3RXorQ3ZVTk1oemliSEk0QkVqdWVuS1RSRXZjMDE5SUhIeFFp?=
 =?utf-8?B?UXdERGR6YWhHdFlUUXZYTWI2eFM5OEhKQjkvRnVvYk9kWk8wMlEvRXdwYjEx?=
 =?utf-8?B?R0p5RDQ1M2hwRDNMM0lxcGgrUlZxV00vWEZLd1hNK010N2MzQkhDL05qUDNS?=
 =?utf-8?B?QVlPK0laemZzdy92ZHpWdVVjRkJJUis2WmFDQ05hOFQrRmpGZUxWaEZPTnB2?=
 =?utf-8?B?R3FiVWFmQUI2RkpkdU1XL2ROR3pHQVVkTjNTS2Y4QTFxYTBOYVloZU5nUDJJ?=
 =?utf-8?B?VEFhcldXRzNGQ1dhOTk3U0lVN3g5cVdWUk9vcTQydlordXEvTXhqanUwOGt2?=
 =?utf-8?B?N2dKdEtsZ3dwcTlWY2VEdU5hVlRHdDRqa2swS1prbm0xRTI5bzl1L1ljZm10?=
 =?utf-8?B?NTZNOGp2ZCtST3g2eVVheWtpbGpFNWR4V1RxY3pDWHExN0czUVB2VCt4TVVV?=
 =?utf-8?B?TGZvU3lLZ3YxdmpRdFcxamtSd0pnZXdXbTgrd2MwNUQ0by81RGthOFBITnlo?=
 =?utf-8?B?SUlZM05wRUtFL2ZzRkV0ekJReGpSUnNMYW42WklQUnBja28zTXFiZlBVcWw5?=
 =?utf-8?B?SHJpbGdJUXhCM2V3ZVY0M1NuR0RyVCtkdHRJY2Y4RWxHd1MyR3ZBQ1FPbUND?=
 =?utf-8?B?TlF1QmwzemxjWk92TmF5dE54enA3TSt5QjJCV3c0LzAxSGhEY1Z0aWtRQTVQ?=
 =?utf-8?B?OGM4azU5YSt3U29NM3ZkWjFGTzE3UUhHQ0xlYmpOMnlzMWNNVEhwd0RXQlVm?=
 =?utf-8?B?RS9QR3BiNGNmU1BVcVhPSXFGbXVyckN4R0tPOFdlc0FjTStYeTFzY3ZFMkNN?=
 =?utf-8?B?bkVoNzM4WjNDNEM2VFNHbUNMMXhSUHA5YnNkSFNzYjNLNlNRWnNvdDhId2w0?=
 =?utf-8?B?MWhvalBDNHN1NW9MUTFpMHVTZ1R3T25sQldmb0didzRUZkNZSDlZM2hrNkxq?=
 =?utf-8?B?V004a3RsY0ZiVno5ckJpZmZGVXVrK3NWMUcxSy95bUVic2k4SjFGMkd3K2Na?=
 =?utf-8?B?RFE9PQ==?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2bee1ee7-e0be-41de-d767-08dc711e2632
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2024 18:22:28.4521
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wd5e9b+6np/IPQHTTqMzZgvnUMf1NNW8aJC1+rJw7Ap33paJkcizDKEYiaX5PC2KT8y307VQ+KwAZhUpvY638w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB4296

On 5/10/24 10:56 AM, John Hubbard wrote:
> On 5/10/24 4:52 AM, Ryan Roberts wrote:
>> On 04/05/2024 05:43, John Hubbard wrote:
> ...
>> It just occured to me that the bug report I was fixing with my attempt 
>> was
>> invoking make like this (see [1]):
>>
>> # tools/testing/selftests/fchmodat2$ make CC=clang
>> # tools/testing/selftests/openat2$ make CC=clang
>>
>> So LLVM is not set in this case. Perhaps my approach [2] (suggested by 
>> Arnd) of
>> using cc-option is more robust? (cc-option is alredy used by other 
>> selftests).
>>
> 
> Yes, I think that would better handle the two cases: setting LLVM,
> and/or setting CC (!).
> 
> For that, some nits, but only worth fussing over if the patch hasn't
> gone in yet, or if you're changing it for some other reason:
> 

I just remembered it needs the LOCAL_HDRS approach as well. Did your
patch already go in? Should I fix up this one here to use cc-option,
or go with yours? Either way is fine with me.

thanks,
-- 
John Hubbard
NVIDIA

> In Make, the arguments to functions include *all* spaces, so it's good
> practice to not add spaces in most function calls, unless they are
> definitely desired.
> 
> Also, you only ever want one of those $(CC) options, so saying so is a
> nice touch. Neither of these is a functional issue in [2], but you could
> do this on top of the patch (I'm only showing the openat2 case):
> 
> diff --git a/tools/testing/selftests/openat2/Makefile 
> b/tools/testing/selftests/openat2/Makefile
> index 02af9b6ca5eb..c894778874a5 100644
> --- a/tools/testing/selftests/openat2/Makefile
> +++ b/tools/testing/selftests/openat2/Makefile
> @@ -3,7 +3,7 @@
>   include ../../../build/Build.include
> 
>   CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined
> -CFLAGS += $(call cc-option, -static-libasan) $(call cc-option, 
> -static-libsan)
> +CFLAGS += $(call cc-option,-static-libasan,$(call 
> cc-option,-static-libsan))
>   TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test
> 
>   include ../lib.mk
> 
> 
>>
>> [1] https://lore.kernel.org/all/202404141807.LgsqXPY5-lkp@intel.com/
>> [2]
>> https://lore.kernel.org/linux-kselftest/20240417160740.2019530-1-ryan.roberts@arm.com/
>>
>>
>>> +
>>> +LOCAL_HDRS += helpers.h
>>> +
>>>   include ../lib.mk
>>> -$(TEST_GEN_PROGS): helpers.c helpers.h
>>> +$(TEST_GEN_PROGS): helpers.c
>>>
>>> base-commit: ddb4c3f25b7b95df3d6932db0b379d768a6ebdf7
>>> prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
>>
> 
> thanks,