setting flag makes Create Function fail

[simeonpilgrim]

Describe the bug A have new new processor that I am developing support for (https://github.com/simeonpilgrim/ghidra_fujitsu_fr).

A leaf function after disassembling the function if I press f (Create Function) I getting an exception pop up.

 2020-05-27 13:45:06 ERROR (ToolTaskManager) Create Function Failed: java.lang.NullPointerException  

If I reduce the code down to a really tiny set of 20 bytes, the issue still happen, thus the code is

1. loads two registers with 32bit values
2. reads from one register a byte from ram
3. shifts the result
4. writes the result word into the other register
5. returns

if I press Create Function on the instruct after the read register is set, it work. or if I change the read register to that the consts (address) is loaded into to not be the read register the Create Function works. or if I alter the shift to not set C or Z flags the Create Function works

The decompiler gets the function correct, just the disassembler is unhappy.

The pcode for the last example that is happy looks like:

|||||||||||||||||||| FUNCTION |||||||||||||||||||||||||||||||
                             FUN_00000000
        00000000 9f 84 9d        ldi:32     DAT_9d319464,r4
                 31 94 64
                                                      r4 = COPY 0x9d319464:4
        00000006 9f 8c 9d        ldi:32     0x9d1c2ce4,r12
                 1c 2c e4
                                                      r12 = COPY 0x9d1c2ce4:4
        0000000c 06 c1           ldub       @r12=>DAT_9d1c2ce4,r1
                                                      $U1330:1 = LOAD ram(r12)
                                                      r1 = INT_ZEXT $U1330
        0000000e b0 61           lsr        0x6,r1
                                                      r1 = INT_RIGHT r1, 6:4
        00000010 9f 20           ret:D
                                                      pc = COPY rp
                                                      STORE ram(r4), r1
                                                      RETURN pc
        00000012 15 41           _sth       r1,@r4=>DAT_9d319464
                                                      STORE ram(r4), r1

The case which is unhappy and I set a flags register explicitly to zero

.sinc

:lsr tc_x, tc_reg_i is tc_opcode=0b10110000 & tc_reg_i & tc_x {
    local shift:4 = tc_x & 0x1f;
    tc_reg_i = tc_reg_i >> shift;
    $(Z_FLAG) = 1;
}

outputs this pcode:

                             //
                             // ram 
                             // ram: 00000000-00000013
                             //
        00000000 9f 84 9d        ldi:32     DAT_9d319464,r4                                  = ??
                 31 94 64
                                                      r4 = COPY 0x9d319464:4
        00000006 9f 8c 9d        ldi:32     0x9d1c2ce4,r12
                 1c 2c e4
                                                      r12 = COPY 0x9d1c2ce4:4
        0000000c 06 c1           ldub       @r12=>DAT_9d1c2ce4,r1                            = ??
                                                      $U1380:1 = LOAD ram(r12)
                                                      r1 = INT_ZEXT $U1380
        0000000e b0 61           lsr        0x6,r1
                                                      $Ue90:4 = INT_AND 6:4, 31:4
                                                      r1 = INT_RIGHT r1, $Ue90
                                                      $Ueb0:4 = INT_AND *[register]0x104:4, 0xfffffffb:4
                                                      $Uec0:4 = INT_ZEXT 1:1
                                                      $Ued0:4 = INT_LEFT $Uec0, 2:4
                                                      *[register]0x104:4 = INT_OR $Ueb0, $Ued0
        00000010 9f 20           ret:D
                                                      pc = COPY rp
                                                      STORE ram(r4), r1
                                                      RETURN pc
        00000012 15 41           _sth       r1,@r4=>DAT_9d319464                             = ??
                                                      STORE ram(r4), r1

and triggers the pop-up.

Expected behavior A function to be created.

Screenshots

Attachments

Environment :

• OS: Windows 10
• Java Version: 12.0.1
• Ghidra Version: 9.1.2

Other comment There are number of exception that a thrown when I do a full analysis, they a super annoying at many levels,

• they are full desktop alerts, so the PC cannot be used for other purposes during this process, as focus get yanked
• they are model, so must be clicked on, to remove,
• they stack 100+ deep (even though they are written to the log, and really a bookmark would be more use)
• they have no context (they have a stack trace) but

Analysis Task: Subroutine References - 
java.lang.NullPointerException
    at ghidra.app.cmd.function.CreateThunkFunctionCmd.containsRegister(CreateThunkFunctionCmd.java:799)
    at ghidra.app.cmd.function.CreateThunkFunctionCmd.addRegisterUsage(CreateThunkFunctionCmd.java:764)
    at ghidra.app.cmd.function.CreateThunkFunctionCmd.getThunkedAddr(CreateThunkFunctionCmd.java:570)
    at ghidra.app.plugin.core.function.FunctionAnalyzer.added(FunctionAnalyzer.java:129)
    at ghidra.app.plugin.core.function.CreateThunkAnalyzer.added(CreateThunkAnalyzer.java:51)
    at ghidra.app.plugin.core.analysis.AnalysisScheduler.runAnalyzer(AnalysisScheduler.java:185)
    at ghidra.app.plugin.core.analysis.AnalysisTask.applyTo(AnalysisTask.java:39)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisTaskWrapper.run(AutoAnalysisManager.java:685)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:785)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:664)
    at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:629)
    at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:62)
    at ghidra.framework.plugintool.mgr.BackgroundCommandTask.run(BackgroundCommandTask.java:101)
    at ghidra.framework.plugintool.mgr.ToolTaskManager.run(ToolTaskManager.java:315)
    at java.base/java.lang.Thread.run(Thread.java:835)

---------------------------------------------------
Build Date: 2020-Feb-12 1149 EST
Ghidra Version: 9.1.2
Java Home: C:\data\jdk-12.0.1
JVM Version: Oracle Corporation 12.0.1
OS: Windows 10 10.0 amd64
Workstation: Simeon2

doesn't tell me what address/subroutine exploded, so I cannot go look at the address and workout is it my code that wrong, or make a minimum report/example.

[simeonpilgrim]

Ok, so I jump some hoops, and got the "ghidra_9.1.2_build" tag building and debugged this:

So the register that it's trying to fetch is the PS (processor state) to clear/set the Zero flag.

If I try set the Registor Context on the first instruction, I get the error message

Context register change conflicts with one or more instructions

which makes some sense, but given the state of the flags is unknown at the start of the processor init this is strange.

[astrelsky]

program.getRegister(regVarnode) is returning null

[simeonpilgrim]

program.getRegister(regVarnode) is returning null

@astrelsky yes, that is point of the change set attached, but I do not understand who your comment is targeted at, and why they need that comment?

[astrelsky]

program.getRegister(regVarnode) is returning null

@astrelsky yes, that is point of the change set attached, but I do not understand who your comment is targeted at, and why they need that comment?

I did not see the linked pull request before submitting the comment. I encountered this myself earlier today and was going to submit an issue but found this one. It's often helpful to know where the null pointer is coming from so I was providing that information. It can clearly be deduced from the pull request though.

[simeonpilgrim]

@astrelsky ah I see, nod I agree the screenshot of the code in question did not explicitly call out the line. And from the context you explain, that makes sense. Thank you for providing that context.

[astrelsky]

@astrelsky ah I see, nod I agree the screenshot of the code in question did not explicitly call out the line. And from the context you explain, that makes sense. Thank you for providing that context.

Of course. Arguably I should have provided more context in the first place.

[simeonpilgrim]

to this cases point, the secondary exception I also listed:

Analysis Task: Subroutine References -

is the same piece of code, and the auto analysis runs much more happy once this change has been applied.

[ryanmkurtz]

Should be fixed with c9121fe49877f14f22ee66bb01d6693ecee44d55.