Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a few #ifdef vs. #if issues #66898

Merged
merged 3 commits into from
Mar 22, 2022
Merged

Fix a few #ifdef vs. #if issues #66898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db70b45
Fix a few #ifdef vs. #if issues
am11 Mar 20, 2022
0203284
Apply libunwind/libunwind#342
am11 Mar 20, 2022
9cf1d2f
Merge dotnet/main into feature/build/native
am11 Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/pal/src/config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
#cmakedefine01 HAVE_SEMAPHORE_H
#cmakedefine01 HAS_SYSV_SEMAPHORES
#cmakedefine01 HAS_PTHREAD_MUTEXES
#cmakedefine01 HAVE_TTRACE
#cmakedefine HAVE_TTRACE
#cmakedefine01 HAVE_PIPE2
#cmakedefine01 HAVE_SCHED_GETAFFINITY
#cmakedefine01 HAVE_SCHED_SETAFFINITY
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/pal/src/debug/debug.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ SET_DEFAULT_DEBUG_CHANNEL(DEBUG); // some headers have code with asserts, so do
#include <unistd.h>
#if HAVE_PROCFS_CTL
#include <unistd.h>
#elif HAVE_TTRACE // HAVE_PROCFS_CTL
#elif defined(HAVE_TTRACE) // HAVE_PROCFS_CTL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we use cmakedefine/defined here over the existing check? The other cases out of libunwind make sense as they were always defined and they are "HAVE" remarks. Is this because libunwind uses this pattern more often?

Copy link
Member Author

@am11 am11 Mar 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, correct.

It is actually related to config.h.in change, we are using same define (pr: #cmakedefine HAVE_TTRACE, main: #cmakedefine01 HAVE_TTRACE) for libunwind and debug.cpp. So I converged it to libunwind's way.

When we use #cmakedefine01 FOO, it generates #define FOO 1 (when feature is detected) or #define FOO 0 (when detection fails), so defined(FOO) is always true and it is a logical error if we use that. With #cmakedefine FOO, it generates #define FOO or nothing when detection fails. In that case #if FOO issues a warning Wundef.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to know why use the define divergent when everything uses the logical one. If it's for libunwind consistency that's all good.

#include <sys/ttrace.h>
#else // HAVE_TTRACE
#else // defined(HAVE_TTRACE)
#include <sys/ptrace.h>
#endif // HAVE_PROCFS_CTL
#if HAVE_VM_READ
Expand Down
4 changes: 2 additions & 2 deletions src/native/eventpipe/ds-ipc-pal-socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,14 +530,14 @@ ipc_socket_accept (
ds_ipc_socket_t client_socket;
DS_ENTER_BLOCKING_PAL_SECTION;
do {
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
#if HAVE_ACCEPT4 && defined(SOCK_CLOEXEC)
client_socket = accept4 (s, address, address_len, SOCK_CLOEXEC);
#else
client_socket = accept (s, address, address_len);
#endif
} while (ipc_retry_syscall (client_socket));

#if !defined(HAVE_ACCEPT4) || !defined(SOCK_CLOEXEC)
#if !HAVE_ACCEPT4 || !defined(SOCK_CLOEXEC)
#if defined(FD_CLOEXEC)
if (client_socket != -1)
{
Expand Down
1 change: 1 addition & 0 deletions src/native/external/libunwind-version.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ https://github.com/libunwind/libunwind/commit/b3ca1b59a795a617877c01fe5d299ab7a0
Reapply changes from https://github.com/dotnet/runtime/commit/1b5719c2e3dde393531eaeb5b5cde05dabeef5b8
Apply https://github.com/libunwind/libunwind/pull/317
Apply https://github.com/libunwind/libunwind/pull/333
Apply https://github.com/libunwind/libunwind/pull/342

For LoongArch64:
Apply https://github.com/libunwind/libunwind/pull/316 and https://github.com/libunwind/libunwind/pull/322
2 changes: 1 addition & 1 deletion src/native/external/libunwind/src/ptrace/_UPT_access_fpreg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */

#include "_UPT_internal.h"

#if HAVE_DECL_PTRACE_POKEUSER || HAVE_TTRACE
#if HAVE_DECL_PTRACE_POKEUSER || defined(HAVE_TTRACE)
int
_UPT_access_fpreg (unw_addr_space_t as, unw_regnum_t reg, unw_fpreg_t *val,
int write, void *arg)
Expand Down
2 changes: 1 addition & 1 deletion src/native/external/libunwind/src/ptrace/_UPT_access_mem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */

#include "_UPT_internal.h"

#if HAVE_DECL_PTRACE_POKEDATA || HAVE_TTRACE
#if HAVE_DECL_PTRACE_POKEDATA || defined(HAVE_TTRACE)
int
_UPT_access_mem (unw_addr_space_t as, unw_word_t addr, unw_word_t *val,
int write, void *arg)
Expand Down
2 changes: 1 addition & 1 deletion src/native/external/libunwind/src/ptrace/_UPT_access_reg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ _UPT_access_reg (unw_addr_space_t as, unw_regnum_t reg, unw_word_t *val,
Debug (1, "bad register %s [%u] (error: %s)\n", unw_regname(reg), reg, strerror (errno));
return -UNW_EBADREG;
}
#elif HAVE_DECL_PTRACE_POKEUSER || HAVE_TTRACE
#elif HAVE_DECL_PTRACE_POKEUSER || defined(HAVE_TTRACE)
int
_UPT_access_reg (unw_addr_space_t as, unw_regnum_t reg, unw_word_t *val,
int write, void *arg)
Expand Down
8 changes: 4 additions & 4 deletions src/native/libs/System.Native/pal_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ static void ConvertFileStatus(const struct stat_* src, FileStatus* dst)
dst->BirthTimeNsec = 0;
#endif

#if defined(HAVE_STAT_FLAGS) && defined(UF_HIDDEN)
#if HAVE_STAT_FLAGS && defined(UF_HIDDEN)
dst->UserFlags = ((src->st_flags & UF_HIDDEN) == UF_HIDDEN) ? PAL_UF_HIDDEN : 0;
#else
dst->UserFlags = 0;
Expand Down Expand Up @@ -1607,7 +1607,7 @@ int32_t SystemNative_LockFileRegion(intptr_t fd, int64_t offset, int64_t length,
struct flock lockArgs;
#endif

#if defined(TARGET_ANDROID) && defined(HAVE_FLOCK64)
#if defined(TARGET_ANDROID) && HAVE_FLOCK64
// On Android, fcntl is always implemented by fcntl64 but before https://github.com/aosp-mirror/platform_bionic/commit/09e77f35ab8d291bf88302bb9673aaa518c6bcb0
// there was no remapping of F_SETLK to F_SETLK64 when _FILE_OFFSET_BITS=64 (which we set in eng/native/configurecompiler.cmake) so we need to always pass F_SETLK64
int command = F_SETLK64;
Expand Down Expand Up @@ -1640,7 +1640,7 @@ int32_t SystemNative_LChflags(const char* path, uint32_t flags)

int32_t SystemNative_LChflagsCanSetHiddenFlag(void)
{
#if defined(HAVE_LCHFLAGS)
#if HAVE_LCHFLAGS
return SystemNative_CanGetHiddenFlag();
#else
return false;
Expand All @@ -1649,7 +1649,7 @@ int32_t SystemNative_LChflagsCanSetHiddenFlag(void)

int32_t SystemNative_CanGetHiddenFlag(void)
{
#if defined(UF_HIDDEN) && defined(HAVE_STAT_FLAGS)
#if HAVE_STAT_FLAGS && defined(UF_HIDDEN)
return true;
#else
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/native/libs/System.Native/pal_mount.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ int32_t SystemNative_GetAllMountPoints(MountPointFound onFound)
{
#if HAVE_MNTINFO
// getmntinfo returns pointers to OS-internal structs, so we don't need to worry about free'ing the object
#if defined(HAVE_STATFS)
#if HAVE_STATFS
struct statfs* mounts = NULL;
#else
struct statvfs* mounts = NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/native/libs/System.Native/pal_networking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,7 @@ int32_t SystemNative_Accept(intptr_t socket, uint8_t* socketAddress, int32_t* so

socklen_t addrLen = (socklen_t)*socketAddressLen;
int accepted;
#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC)
#if HAVE_ACCEPT4 && defined(SOCK_CLOEXEC)
while ((accepted = accept4(fd, (struct sockaddr*)socketAddress, &addrLen, SOCK_CLOEXEC)) < 0 && errno == EINTR);
#else
while ((accepted = accept(fd, (struct sockaddr*)socketAddress, &addrLen)) < 0 && errno == EINTR);
Expand Down