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

Jump to bottom

Building with clang-cl #625

Closed
gvanem opened this issue Aug 20, 2022 · 2 comments
Closed

Building with clang-cl #625

gvanem opened this issue Aug 20, 2022 · 2 comments

Comments

@gvanem
Copy link

gvanem commented Aug 20, 2022

When using clang-cl to compile the NPcap sources, there are some C++ errors that MSVC's compiler
does not even warn about even on -W4. Like this:

BOOLEAN PacketGetNetInfoEx(PCCH AdapterName, npf_if_addr* buffer, PLONG NEntries)
{
   ...
	if (!AdapterName || !buffer || NEntries <= 0) {   // !! here

Error:

./NPcap/packetWin7/Dll/Packet32.cpp(3220,42): error: ordered comparison between pointer and zero ('PLONG' (aka 'long *') and 'int')
        if (!AdapterName || !buffer || NEntries <= 0) {
                                       ~~~~~~~~ ^  ~

Surely it should be:

	if (!AdapterName || !buffer || *NEntries <= 0) {

A bug that surfaces depending on the raw location of *NEntries.

And then there are these errors:

./NPcap/packetWin7/Dll/Packet32.cpp(3672,8): error: case value evaluates to -1073676267, which cannot be narrowed to type 'DWORD'
      (aka 'unsigned long') [-Wc++11-narrowing]
                case NDIS_STATUS_INVALID_DATA:
                     ^
./NPcap/packetWin7/Dll/Packet32.cpp(3663,49): note: expanded from macro 'NDIS_STATUS_INVALID_DATA'
#define NDIS_STATUS_INVALID_DATA                ((NDIS_STATUS)0xC0010015L)
                                                ^
./NPcap/packetWin7/Dll/Packet32.cpp(3673,8): error: case value evaluates to -1073676265, which cannot be narrowed to type 'DWORD'
      (aka 'unsigned long') [-Wc++11-narrowing]
                case NDIS_STATUS_INVALID_OID:
                     ^
./NPcap/packetWin7/Dll/Packet32.cpp(3664,49): note: expanded from macro 'NDIS_STATUS_INVALID_OID'
#define NDIS_STATUS_INVALID_OID                 ((NDIS_STATUS)0xC0010017L)
                                                ^

Since NDIS_STATUS_INVALID_DATA == (NDIS_STATUS)0xC0010015L expands to (int) -1073676267 that's not suitable for a DWORD.
But with this patch, it compiles:

--- a/NPcap/packetWin7/Dll/Packet32.cpp 2022-08-20 12:20:53
+++ b/NPcap/packetWin7/Dll/Packet32.cpp 2022-08-20 08:27:46
@@ -3660,8 +3660,8 @@
        dwResult = PacketRequestHelper(hAdapter, TRUE, OidData);

 #ifndef NDIS_STATUS_INVALID_DATA
-#define NDIS_STATUS_INVALID_DATA                ((DWORD)0xC0010015UL)
-#define NDIS_STATUS_INVALID_OID                 ((DWORD)0xC0010017UL)
+#define NDIS_STATUS_INVALID_DATA                ((NDIS_STATUS)0xC0010015L)
+#define NDIS_STATUS_INVALID_OID                 ((NDIS_STATUS)0xC0010017L)
 #endif
        switch (dwResult)
        {

Or adding -Wno-c++11-narrowing.

I've not yet tried to use clang-cl for the driver code itself. I suspect more such issues there.
@dmiller-nmap
Copy link
Contributor

This is a good catch on the NEntries issue. Most of the other things I see are C++-isms that would be good ideas, but would fit better under a complete rewrite to C++. I'm not entirely certain why the choice was made to move from C to C++ for Packet.dll, but the driver is still C. Clang does not seem to be able to understand the WDK headers, and so cannot actually build the driver, but it did find a few unused variables that I have now removed.

@gvanem
Copy link
Author

gvanem commented Aug 22, 2022

Clang does not seem to be able to understand the WDK headers

Yes it does. I've built System Informer kernel with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gvanem @dmiller-nmap