Jump to content

Clover problems report & features request

ErmaC

By ErmaC,
in Clover

 Share
953 posts in this topic

Recommended Posts

TheRacerMaster
TheRacerMaster
Posted
Posted (edited)

32-bit builds appear to be broken with GCC 8: 

Clover/rEFIt_UEFI/Platform/AcpiPatcher.c:222:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
       gBS->FreePages((EFI_PHYSICAL_ADDRESS)XsdtEntryFromIndex(Index), XsdtReplaceSizes[Index]);
                      ^
#define XsdtEntryFromIndex(index) (EFI_ACPI_DESCRIPTION_HEADER*)(UINTN)ReadUnaligned64(XsdtEntryPtrFromIndex(index))

EFI_PHYSICAL_ADDRESS is 64-bit, so this is casting from a 32-bit pointer to a 64-bit integer.

 

Also, r4482's plist.c changes (specifically, the "end of cache" freeing) appear to cause memory corruption. Removing this fixes kext injection issues (plist.c memory corruption causes some kext versions to display <null string> in bdmesg).

Edited by TheRacerMaster
  • Like 2
Link to comment
Share on other sites
Slice
Slice
Posted
Posted
1 hour ago, TheRacerMaster said:

32-bit builds appear to be broken with GCC 8: 


Clover/rEFIt_UEFI/Platform/AcpiPatcher.c:222:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
       gBS->FreePages((EFI_PHYSICAL_ADDRESS)XsdtEntryFromIndex(Index), XsdtReplaceSizes[Index]);
                      ^

#define XsdtEntryFromIndex(index) (EFI_ACPI_DESCRIPTION_HEADER*)(UINTN)ReadUnaligned64(XsdtEntryPtrFromIndex(index))

EFI_PHYSICAL_ADDRESS is 64-bit, so this is casting from a 32-bit pointer to a 64-bit integer.

 

Also, r4482's plist.c changes (specifically, the "end of cache" freeing) appear to cause memory corruption. Removing this fixes kext injection issues (plist.c memory corruption causes some kext versions to display <null string> in bdmesg).

Thanks,

STATIC arrays accepted in 4484,

plist.c fixed in 4483.

EFI_PHYSICAL_ADDRESS will be fixed.  EDITED: 4485

  • Like 2
Link to comment
Share on other sites
Mr MagOO
Mr MagOO
Posted
Posted

Hi,

it appears that 

        <key>FakeID</key>
        <dict>
            <key>LAN</key>
            <string>0x435411ab</string>
        </dict>

is broken since rev 4497

OK with 4495.

Is it a bug in new injection method ?

 

Link to comment
Share on other sites
vit9696
vit9696
Posted
Posted

Zenith432, why did you change architecture 32+64 to 64 in clover utilities? This breaks 32-bit compilation I still use. 10.14 has nothing to do to fat binaries, please revert the change back.

  • Like 1
Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted (edited)
9 hours ago, vit9696 said:

Zenith432, why did you change architecture 32+64 to 64 in clover utilities? This breaks 32-bit compilation I still use. 10.14 has nothing to do to fat binaries, please revert the change back.

Explained here

Which compilation did it break?

 

FWIW: I know why 32-bit build of clover-genconfig defines MDE_CPU_X64.  It's because in project.pbxproj, HEADER_SEARCH_PATHS is always MdePkg/Include/X64, so ProcessorBind.h is taken from there and code always uses 64 EDK2 defs.  In particular, the lines

Quote
#if defined(MDE_CPU_IA32)
printf("32 bit generator\n");
#elif defined(MDE_CPU_X64)
printf("64 bit generator\n");

in clover-genconfig.c are a joke.

 

PS: So it seems even 32-bit build of clover-genconfig wants to parse binary format of SETTINGS_DATA used by 64-bit Clover.  Is this right?  Are we looking for version to parse SETTINGS_DATA for 32-bit Clover?  Should 32-bit support be fully discontinued?  I need to know what the intention was to decide on resolution.

 

PPS: Including X64 version of ProcessorBind.h is 32-bit build of clover-genconfig cannot work anyway.  The struct SETTINGS_DATA has pointers in it, which are a different size, so 32-bit build will not end up with correct structure for 64-bit Clover.

 

@Slice: did you bother to test new code in r4501?  I see in main() in clover-genconfig.c call to GetOFVariable("device-properties", ...) which uses global gPlatform before it's been initialized.

Edited by Zenith432
  • Like 2
Link to comment
Share on other sites
vit9696
vit9696
Posted
Posted

@Zenith432, the obvious issue here is that you enforced clover-genconfig being 64-bit, and thus it does not run on a 32-bit only CPU (and yes, that is my case, which I still rarely use for testing some really old legacy projects). So it is very clear how and why this is broken :).

 

Sure, nobody did request you to support a 32-bit only binary (which I agree is inadequate), yet a fat binary should successfully be able to generate a 32-bit config on a 32-bit only system, and a 64-bit config on a 64-bit system. For hybrid ones there have always been an arch util (man arch).

 

To fix the actual header paths bug you need to change the Xcode project header inclusion paths. If you click on + in Xcode project prefs it would let you define different header paths depending on the architecture. This should have been done right previously, yet some crazy attempts to make it "64-bit only" must have borked it.

 

Thanks~

  • Like 3
Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted

The 32-bit build of clover-genconfig didn't work before.  It was built, but it didn't do anything. It used arch i386 and X64/ProcessorBind.h which gives a different sizeof(SETTINGS_DATA) than compiling with arch x86_64.  In clover-genconfig.c there is a sanity check for sizeof(SETTINGS_DATA) being the same as the size of the data being parsed and it exited with an error message.

The reason for removal of 32-bit build was that after reversion of MdePke/Include/Base.h, it needs MDE_CPU_IA32 for arch i386 or it generates a compile error.  Right now, both xcodeproj and Makefile for clover-genconfig do not result in any errors, but do not build 32-bitnary either.

I'll only have access to macos on Monday, but when I do I'll change xcodeproj to use IA32/ProcessorBind.h for arch i386, return 32-bit into fat binary.  It is the only workable solution, because hybrid option does not yield a usable program.

  • Like 1
  • Thanks 1
Link to comment
Share on other sites
Slice
Slice
Posted
Posted
On 5/26/2018 at 8:19 AM, Zenith432 said:

 

@Slice: did you bother to test new code in r4501?  I see in main() in clover-genconfig.c call to GetOFVariable("device-properties", ...) which uses global gPlatform before it's been initialized.

4501 is not working version. I commit it to have an ability get my draft from other place.

I want to seriously advance clover-genconfig.

Link to comment
Share on other sites
Mr MagOO
Mr MagOO
Posted
Posted
On 5/26/2018 at 2:28 AM, Mr MagOO said:

Hi,

it appears that 

        <key>FakeID</key>
        <dict>
            <key>LAN</key>
            <string>0x435411ab</string>
        </dict>

is broken since rev 4497

OK with 4495.

Is it a bug in new injection method ?

 

@Slice : is there a chance the FakeID injection is broken or do I need to change syntax or something to make it work post rev 4496 ?

Link to comment
Share on other sites
Slice
Slice
Posted
Posted
6 hours ago, Mr MagOO said:

@Slice : is there a chance the FakeID injection is broken or do I need to change syntax or something to make it work post rev 4496 ?

Yes, you have to add

Quote

    <key>Devices</key>
    <dict>

        <key>LANInject</key>
        <true/>

 

Sorry for the back incompatibility.

I have to disable automatic injection to not mix with new method of Properties injection.

  • Like 2
  • Thanks 1
Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted

@Slice

The latest config-sample.plist you committed has embedded nulls in it.  For example, after the string "Apple WiFi card".  There are others.

Also, the file was previously sorted.  It makes it hard to merge changes when each commit is randomly sorted from previous one.

Link to comment
Share on other sites
Mr MagOO
Mr MagOO
Posted
Posted (edited)
15 hours ago, Slice said:

Yes, you have to add

Sorry for the back incompatibility.

I have to disable automatic injection to not mix with new method of Properties injection.

 

Well, something is really happening with LANInjection=true ...

but I don't know what because I lost my video with a black screen boot :)

I've got this in Devices and Graphics : 

	<key>Devices</key>
	<dict>
		<key>Audio</key>
		<dict>
			<key>Inject</key>
			<string>no</string>
		</dict>
        <key>LANInjection</key>
		<true/>
		<key>FakeID</key>
		<dict>
			<key>LAN</key>
			<string>0x435411ab</string>
		</dict>
		<key>USB</key>
		<dict>
			<key>AddClockID</key>
			<true/>
			<key>FixOwnership</key>
			<true/>
			<key>HighCurrent</key>
			<true/>
			<key>Inject</key>
			<true/>
		</dict>
	</dict>

	<key>Graphics</key>
	<dict>
		<key>Inject</key>
		<dict>
			<key>ATI</key>
			<false/>
			<key>Intel</key>
			<false/>
			<key>NVidia</key>
			<true/>
		</dict>
		<key>NvidiaGeneric</key>
		<true/>
		<key>VideoPorts</key>
		<integer>3</integer>
	</dict>

Is there any change for video injection ?

edit : {censored}, my fault, all is OK now with rev 4507.

Thank you Slice!

 

Edited by Mr MagOO
Link to comment
Share on other sites
Slice
Slice
Posted
Posted
3 hours ago, Zenith432 said:

@Slice

The latest config-sample.plist you committed has embedded nulls in it.  For example, after the string "Apple WiFi card".  There are others.

Also, the file was previously sorted.  It makes it hard to merge changes when each commit is randomly sorted from previous one.

It is PlistEditPro mischief. I corrected this in 4507. I will no more use this strange application.

Link to comment
Share on other sites
Slice
Slice
Posted
Posted
5 hours ago, Zenith432 said:

@Slice

The latest config-sample.plist you committed has embedded nulls in it.  For example, after the string "Apple WiFi card".  There are others.

 

It looks like nulls are known problem

Quote

  CopyMem(configBuffer, buffer, bufferSize);
  for (i=0; i<bufferSize; i++) {
    if (configBuffer == 0) {
      configBuffer = 0x20;  //replace random zero bytes to spaces
    }
  }

 

But I am not sure if this is good solution. Anyway we live with this workaround some years.

Link to comment
Share on other sites
Slice
Slice
Posted
Posted
13 hours ago, Zenith432 said:

@Slice

The latest config-sample.plist you committed has embedded nulls in it.  For example, after the string "Apple WiFi card".  There are others.

 

clover-genconfig corrected to not produce nulls. rev 4508.

 

Sorry about 32 bits, Xcode 9.3.1 don't like it.

Link to comment
Share on other sites
Slice
Slice
Posted
Posted

A kext produced with SDK10.13 will not work in 10.12 and early. VoodooHDA for example.

I am not sure about applications but I prefer to have SDK10.11 which is noticably better.

 

About clover-genconfig. It uses SETTINGS_DATA structure which can deviate between revisions. So clover-genconfig will work only with the same clover revision.

Moreover clover-genconfig must be 64 bit if clover-64 used for the same reason.

It is possible to run OSX 10.7.5-32 using Clover-64 and in this case clover-genconfig-64 should be used even in system-32.

And yes, 32-bit only computer requires Clover-32 and macOS 10.6, not more, and all 32-bit application. Looks like museum exhibit.

Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted (edited)
22 minutes ago, Slice said:

And yes, 32-bit only computer requires Clover-32 and macOS 10.6, not more, and all 32-bit application. Looks like museum exhibit.

It was requested

https://www.insanelymac.com/forum/topic/329777-clover-problems-report-features-request/?do=findComment&comment=2615056

 

https://www.insanelymac.com/forum/topic/329777-clover-problems-report-features-request/?do=findComment&comment=2615112

 

I thought you meant it doesn't compile.

 

PS: Why are links to specifc posts on new forum broken?

Edited by Zenith432
Link to comment
Share on other sites
 Share
Go to topic listing
×
×
  • Create New...