[SharedCache] Avoid data memory region name conflicts & reduce section naming ones

[WeiN76LQh]

The problems this commit addresses is that Binary Ninja does not allow for sections with duplicate names; the sections are added but they are unnamed. It also does not allow memory map data regions to be added with the same name and will throw an error when attempting to do so.

Prior to this commit the DSC plugin would use the base name of the image path plus the segment name, as a name for a segment region when it is added to the binary view memory map when loading an image. The problem with this is that some images in the DSC share base names, i.e. TSUtility. To avoid this conflict the full image path is used followed by the segment name for memory map data region names.

Additionally this same problem occurs with sections. Prior to this commit the DSC plugin used the image base name plus the section name, however even within an image there can be multiple sections with the same name. This commit changes it so that the segment name is also included in the section name. Analysis shows that images do not contain duplicate named segments and segments do not contain duplicate named sections.

The only problem remaining is the issue of duplicate image base names used in section names. So section names can still be a problem if images with the same base name are loaded. I filed issue #6411 to request sections can have duplicate names. In part this commit actually mitigates half the issue that made me file the feature request but it doesn't seem unreasonable to add support for duplicate section names and it will help fully fix the remaining issue. Arguably there another solution could be provided here for duplicate image base names, like including the image index. Although that information is not particularly useful to the user, but it does differentiate the sections that belong to a given image. I don't think it makes sense to include the full image path in section names because its overly verbose although it does inform the user the most, its just too much text.

[WeiN76LQh]

Section names look like this with this PR applied

[WeiN76LQh]

Need to make some more changes to support the new naming scheme. Things that rely on sections names being formatted in a certain way won't work.

[WeiN76LQh]

I think this is ready to go now

[emesare]

I think latest dev will have less conflicts now that the imageless regions include their start address, however images that share a name with another will still conflict.

I think the best way to fix that one is to just try adding the image section and if it fails because of name then we just prepend the start address

[WeiN76LQh]

The main issue this PR addresses is the conflicts with section names. Checking the latest dev 5.0.7159-dev (70a27b05) it seems that section name conflicts is still a problem. Within an image itself it has multiple sections that have the same name but are part of different segments. For instance an image can have multiple __const sections. I think my PR adequately addresses the issue with a nice readable format involving the segment name as part of the section name. Something that LLDB does.

For images with duplicate names you could do what you suggest and prepend the load address but I think thats kind of ugly. Given section naming conflicts are valid, from what I can tell, implementing #6411 would be the preferred solution to conflicting image names.

[emesare]

I have gone ahead and replicated your change for image section names to include the segments with commit: f902f22

With that being done I think we just have the issue of duplicate image names? We really need to support duplicate section names :/

[bdash]

f902f22 broke Objective-C metadata parsing since it relies on sections being named in a specific way:

binaryninja-api/objectivec/objc.cpp

Lines 1122 to 1132 in 73e7d21

	Ref<Section> ObjCProcessor::GetSectionForImage(std::optional<std::string> imageName, const char* sectionName)
	{
	if (imageName)
	{
	return m_data->GetSectionByName(*imageName + "::" + sectionName);
	}
	else
	{
	return m_data->GetSectionByName(sectionName);
	}
	}

[emesare]

I committed this and in the back of my head "surely no one relies on this behavior".

😮‍💨 Need to make an issue to remove the image name concept from the objective c processor.

But for now ill just write a quick fix to include the region.

[emesare]

I just went ahead and pushed a45466f I thought the image name was more prolific in the objective-c processor but it turned out to be easy enough to just make a getter for sections to override in the shared cache processor.

I think that is the last divergence in the objective-c base processor (divergence between MACH-O and shared cache views).