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

[SDK] CORE-12596 fix NT6 building issues. #356

Merged
merged 1 commit into from
Feb 10, 2018
Merged

[SDK] CORE-12596 fix NT6 building issues. #356

merged 1 commit into from
Feb 10, 2018

Conversation

Getequ
Copy link
Contributor

@Getequ Getequ commented Feb 4, 2018 •
edited
Loading

Purpose

fixing issue and some more SDK headers NT6 improvements

JIRA issue: CORE-12596

@binarymaster
Copy link
Member

Better commit name:

[SDK] Fix NT6 building issues and improve SDK NT6 headers

CORE-12596

@Getequ Getequ force-pushed the CORE-12596 branch 2 times, most recently from 8eca0b9 to 0fd10b8 Compare February 4, 2018 16:17
ThFabba
ThFabba reviewed Feb 5, 2018
View reviewed changes
sdk/include/ndk/peb_teb.h Outdated
@@ -357,11 +357,6 @@ typedef struct STRUCT(_TEB)
UCHAR IdealProcessor;
};
};
#elif (NTDDI_VERSION >= NTDDI_LONGHORN)
Copy link
Member

Choose a reason for hiding this comment

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

I don't have a Vista kernel to check with, but Win7 uses the version above in the >= NTDDI_WIN10 block. What information is your change based on?

Copy link
Contributor Author

@Getequ Getequ Feb 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I not found no any usage of SpareBool0-SpareBool2 and from thier names it looks like stubs for unknown flags just to keep offset for IdealProcessor.
In all places InDbgPrint and next two are used. Also building with winver=0x600 failed because of lack InDbgPrint field.

Maybe comment out this NTDDI_LONGHORN block now instead of deleting?

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I checked, and this block is correct for Vista. However the WIN10 check above should say WIN7 as I mentioned.
The InDbgPrint and HasFiberData members do not exist in newer versions. The DbgInDebugPrint and DbgHasFiberData in SameTebFlags below seem to be their replacements (they don't have the Dbg- prefix though)

   +0x17ee SameTebFlags     : Uint2B
   +0x17ee SafeThunkCall    : Pos 0, 1 Bit
   +0x17ee InDebugPrint     : Pos 1, 1 Bit
   +0x17ee HasFiberData     : Pos 2, 1 Bit
   +0x17ee SkipThreadAttach : Pos 3, 1 Bit
   +0x17ee WerInShipAssertCode : Pos 4, 1 Bit
   +0x17ee RanProcessInit   : Pos 5, 1 Bit
   +0x17ee ClonedThread     : Pos 6, 1 Bit
   +0x17ee SuppressDebugMsg : Pos 7, 1 Bit
   +0x17ee DisableUserStackWalk : Pos 8, 1 Bit
   +0x17ee RtlExceptionAttached : Pos 9, 1 Bit
   +0x17ee InitialThread    : Pos 10, 1 Bit
   +0x17ee SpareSameTebBits : Pos 11, 5 Bits

This means the absence of InDbgPrint & friends is the desired behavior, as the structure must be compatible with Windows. If the kernel doesn't compile with NTDDI_VERSION=NTDDI_VISTA, I'm not sure I'd even consider that a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

damn, you right( ok, roll it back

found it on http://terminus.rewolf.pl/terminus/structures/ntdll/_TEB_combined.html

ThFabba
ThFabba reviewed Feb 5, 2018
View reviewed changes
sdk/include/ndk/i386/asm.h Outdated
//
// HYPERVISOR
//
#define HV_MMU_USE_HYPERCALL_FOR_ADDRESS_SWITCH 0x01
Copy link
Member

Choose a reason for hiding this comment

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

You're in the middle of CPU-defined flags here. I think it would make more sense elsewhere, e.g. above the CPU feature flags starting in line 640/659

ThFabba
ThFabba reviewed Feb 5, 2018
View reviewed changes
sdk/include/ndk/i386/asm.h Outdated
@@ -471,6 +471,17 @@ Header Name:
#endif
#define EFLAGS_USER_SANITIZE 0x3F4DD7


//
// HYPERVISOR
Copy link
Member

Choose a reason for hiding this comment

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

The description doesn't need to be all-caps, and can be a bit more elaborate. E.g. "Hypervisor enlightenment flags", or "Hypervisor Enlightenment Definitions" as is used in kd386.template.h

@Getequ Getequ force-pushed the CORE-12596 branch 2 times, most recently from 8434104 to dbb1b56 Compare February 6, 2018 07:00
SergeGautherie
SergeGautherie reviewed Feb 7, 2018
View reviewed changes
sdk/include/ndk/peb_teb.h Outdated
BOOLEAN SpareBool0;
BOOLEAN SpareBool1;
BOOLEAN SpareBool2;
UCHAR IdealProcessor;
UCHAR IdealProcessor;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unwanted extra space.

sdk/include/asm/ksx.template.h Outdated
@@ -797,7 +797,9 @@ SIZE(KernelThreadObjectLength, KTHREAD),
HEADER("KTIMER"),
OFFSET(TiType, KTIMER, Header.Type),
OFFSET(TiSize, KTIMER, Header.Size),
OFFSET(TiInserted, KTIMER, Header.Inserted), // not in win 10
#if (NTDDI_VERSION < NTDDI_WIN7)
OFFSET(TiInserted, KTIMER, Header.Inserted), // not in win 7
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth keeping the comment?

@Getequ Getequ force-pushed the CORE-12596 branch 2 times, most recently from 139ec23 to 4020eeb Compare February 9, 2018 17:02
@Getequ
Copy link
Contributor Author

Getequ commented Feb 9, 2018 •
edited
Loading

updated winuser.h and fcb.h

@Getequ Getequ force-pushed the CORE-12596 branch 2 times, most recently from b4d8e0a to 2386cbc Compare February 9, 2018 18:25
ThFabba
ThFabba reviewed Feb 10, 2018
View reviewed changes
sdk/include/psdk/winuser.h Outdated
@@ -3446,6 +3446,14 @@ typedef struct tagSOUNDSENTRYW {
DWORD iWindowsEffectOrdinal;
} SOUNDSENTRYW,*LPSOUNDSENTRYW;

#if(_WIN32_WINNT >= 0x0600)
typedef struct tagAUDIODESCRIPTION {
UINT cbSize; // sizeof(AudioDescriptionType)
Copy link
Member

Choose a reason for hiding this comment

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

Copying from Microsoft's headers is not okay. Please delete this, type it yourself (without the comments), and match the formatting in the rest of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, i thought we can use public headers and what can be found on MSDN.

Copy link
Member

@gigaherz gigaherz Feb 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

We can learn from publicly available information, but copying without permission is still copyright infringement.

@ThFabba ThFabba self-assigned this Feb 10, 2018
@ThFabba ThFabba merged commit 420c036 into reactos:master Feb 10, 2018
@Getequ Getequ deleted the CORE-12596 branch February 11, 2018 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gigaherz gigaherz gigaherz left review comments

@ThFabba ThFabba ThFabba left review comments

@SergeGautherie SergeGautherie SergeGautherie left review comments

Assignees

@ThFabba ThFabba

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Getequ @binarymaster @gigaherz @ThFabba @SergeGautherie